Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reorganizing code to current standards #6698

Merged
merged 2 commits into from
Aug 10, 2021

Conversation

jbrider
Copy link
Contributor

@jbrider jbrider commented Aug 10, 2021

working on #572

@hol353 Finally getting back to sorghum - I have reorganised the order according to the coding standards spec, which has created a lot of changes. It should go green, if so, can you merge it.
There are more changes to come but until this one is merged it will be hard to see what those changes are (trying to make it a bit easier).
I have mainly moved code around, changed the readonly getters to use lambdas and changed some public to private.
I still have to fix some capitalisations.
There are a few private variables in the wrong place - they will be extracted out in the next stage.
I will close the old pull request once I have made sure I have captured all the changes from it.

@jbrider
Copy link
Contributor Author

jbrider commented Aug 10, 2021

@hol430 @hol353 Jenkins seems to be having issues with an AssemblyVersion.cs. Is this something I have to work out on my end?

@jbrider
Copy link
Contributor Author

jbrider commented Aug 10, 2021

retest this please jenkins

@hol430
Copy link
Contributor

hol430 commented Aug 10, 2021

Yep - you'll need to edit your initial comment and add "working on #x" (with an issue number). One of the jobs on my list is to get ApsimBot to post a comment to this effect when it spots a PR missing this bit.

@jbrider
Copy link
Contributor Author

jbrider commented Aug 10, 2021

Oops. It has been a while...

@jbrider
Copy link
Contributor Author

jbrider commented Aug 10, 2021

retest this please jenkins

@jbrider
Copy link
Contributor Author

jbrider commented Aug 10, 2021

retest this please jenkins

1 similar comment
@jbrider
Copy link
Contributor Author

jbrider commented Aug 10, 2021

retest this please jenkins

@jbrider
Copy link
Contributor Author

jbrider commented Aug 10, 2021

@hol353 this is right to merge - and will make the next PR much easier to follow.

@hol430
Copy link
Contributor

hol430 commented Aug 10, 2021

@jbrider - what's the status on #6166 - should it be closed?

@jbrider
Copy link
Contributor Author

jbrider commented Aug 10, 2021

@hol430 It can be closed.

@hol353 hol353 merged commit f8156ae into APSIMInitiative:master Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants