-
Notifications
You must be signed in to change notification settings - Fork 119
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
[develop]: Update ConfigWorkflow chapter to HEAD of develop #915
[develop]: Update ConfigWorkflow chapter to HEAD of develop #915
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gspetro-NOAA - Thank you for making the previously suggested changes! I've found two spelling issues, and I had a question regarding the VX_NDIGITS_ENSMEM_NAMES
variable (should the example show the same values, such as "mem002" to "mem02"
). Once these have been addressed, I'll give my approval.
docs/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
docs/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
@mkavulich - During the SRW App CM meeting on September 21, you offered to review the documentation updates in this PR. I'm checking to see if you are still planning on reviewing these changes. If you are, then I will hold off on approving and merging this PR until you have had the opportunity to review these changes. Otherwise, I will go ahead, give my approval, and merge this PR. Please let me know as soon as possible whether you would like to review these changes or not. Thank you very much! |
@MichaelLueken @gspetro-NOAA apologies for letting this slip! I'm reviewing now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this big effort @gspetro-NOAA, this looks like it was a lot of work! Sorry the review took so long, and I admit that I didn't deeply read all the wording changes towards the end, mostly concentrating on the variable names for correctness.
Only a couple of these are corrections, most of these are optional changes so feel free to rebut my suggestions.
docs/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
docs/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
docs/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
docs/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
…pp into text/config
Co-authored-by: Michael Lueken <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
…pp into text/config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing those comments, and opening the new issue #922
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gspetro-NOAA - Thanks for working through my last set of suggested changes! Approving now.
DESCRIPTION OF CHANGES:
This PR updates the ConfigWorkflow.rst file to align with the current state of config_defaults.yaml in develop.
This PR also adds in-code documentation for undocumented variables in config_defaults.yaml.
There are a few odds & ends in other parts of the documentation to fix errors or make updates that came into develop very recently.
Type of change
TESTS CONDUCTED:
None required. Human-readable version of my fork docs available on RTD: https://srw-ug.readthedocs.io/en/text-config/
DEPENDENCIES:
N/A
DOCUMENTATION:
N/A (all docs)
ISSUE:
Issue #879
CHECKLIST