Compiler Warnings Clean up

[HenryMigo]

Have done some clean-up within the code to remove compiler warnings.

I have implemented constructors where I can and ignore warnings as a last resort. In some cases we need to make sure where we don't allow NULL we have only constructors for that model, otherwise more NULL checking needs to happen within the code.

Outstanding Warnings:

• 3692 relate to XML out of the 3708
• 16 are other warnings

Relates to issue: #163

[HenryMigo]

It ended up getting quite big so I drew the line here.

[Craftplacer]

Hey @HenryMigo, congrats on your first pull request.

I've reviewed it and found several issues, I'd like you to take a look at them and resolve them with us.

Due to the size of the pull request I'm going to request reviews from some of our team members.

[Craftplacer]

• 3692 relate to XML out of the 3708

Tracked in #79

[HenryMigo]

I have made the changes as per suggestions, let me know what you think

[Naamloos]

I have some small changes I'd like to see. I added some comments. I'll approve the PR because hacktober is ending soon but consider this a "request changes".

[HenryMigo]

I will address the comments on this PR this evening if that's alright with everyone?

[Naamloos]

I will address the comments on this PR this evening if that's alright with everyone?

sure, no pressure

[Seb-stian]

Job well done.

[HenryMigo]

Just wanted to check that someone is able to complete this PR?

[HenryMigo]

@Craftplacer how do I get this PR completed?

[Craftplacer]

I'm almost satisfied with the changes, the only issue I still see is that the constructor still requires all values to be set (related to Naam's review).

I'd suggest making that the only constructor only requires and sets Name, as to the other values can be set using [Plugin(name, Version = "1.0.0")] etc.

[HenryMigo]

I'm almost satisfied with the changes, the only issue I still see is that the constructor still requires all values to be set (related to Naam's review).

I'd suggest making that the only constructor only requires and sets Name, as to the other values can be set using [Plugin(name, Version = "1.0.0")] etc.

I can make that change but this will bring back the compiler warning asking us to change the props to nullable if they are not in the constructor.

[Craftplacer]

I'm almost satisfied with the changes, the only issue I still see is that the constructor still requires all values to be set (related to Naam's review).

I'd suggest making that the only constructor only requires and sets Name, as to the other values can be set using [Plugin(name, Version = "1.0.0")] etc.

I can make that change but this will bring back the compiler warning asking us to change the props to nullable if they are not in the constructor.

You should make the properties that aren't required nullable, let me know about any issues you encounter doing that.

[HenryMigo]

I'm almost satisfied with the changes, the only issue I still see is that the constructor still requires all values to be set (related to Naam's review).
I'd suggest making that the only constructor only requires and sets Name, as to the other values can be set using [Plugin(name, Version = "1.0.0")] etc.

I can make that change but this will bring back the compiler warning asking us to change the props to nullable if they are not in the constructor.

You should make the properties that aren't required nullable, let me know about any issues you encounter doing that.

Changes made :)

[Craftplacer]

Aight