Update contributing.md for namespace#3353
Merged
chris-ashe merged 4 commits intomainfrom Oct 30, 2024
Merged
Conversation
- Update pull_request_template.md to include following PROCESS style guide - Update CONTRIBUTING.md to ensure implementation follows PROCESS style guide for names of variables and functions Update development standards documentation - Add information about Python PEP8 style guide and its compatibility with Fortran90 - Add guidelines for line length and double declarations - Add guidelines for naming conventions of functions, switches, constants, and variables - Add guidelines for length, physical type, coordinates and dimensions, and units - Add guidelines for docstrings and code documentation using FORD Update mkdocs.yml - Update navigation structure by renaming "Reference" section to "Models" and reorganizing sub-sections
- Updated the standards.md file to clarify the naming conventions for variables, including the use of underscores and capital letters for unit differentiations. - Added a prefix "f_" for variables representing fractions. - Included a reminder to use SI units unless otherwise stated. - Added a section on type hints and referenced PEP-484 for guidance. - Updated the docstring templates for functions and classes. - Added guidelines for comments. Fixes #940
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3353 +/- ##
==========================================
+ Coverage 27.11% 28.00% +0.88%
==========================================
Files 76 76
Lines 17736 18789 +1053
==========================================
+ Hits 4810 5261 +451
- Misses 12926 13528 +602 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jonmaddock
requested changes
Oct 25, 2024
Contributor
jonmaddock
left a comment
jonmaddock
left a comment
There was a problem hiding this comment.
Thanks for doing this Chris, I appreciate it. Some minor clarifications please.
| ### Switches | ||
|
|
||
| Switches should start with the `i_` prefix in their name, be of integer type and should be indexed from 0. | ||
| Switches should by their nature not override other switches. |
Contributor
There was a problem hiding this comment.
Could you clarify this? There are current cases (and legitimate future ones) where defining a switch as one value means that others are ignored or are invalid. Perhaps something could be stated about these scenarios being caught, so switches aren't tacitly ignored in this case, but instead should throw exceptions?
- Adjusted the maximum line length to 79 characters, following PEP8 recommendations, to improve readability and prevent scrolling. - Updated naming conventions for switches and constants. - Added guidelines for variable names related to fractions and constraint equations. - Provided documentation style guidelines, including the use of Google-style docstrings and recommended VSCode extensions.
jonmaddock
requested changes
Oct 28, 2024
Contributor
jonmaddock
left a comment
jonmaddock
left a comment
There was a problem hiding this comment.
Brilliant Chris, I think you've addressed everything apart from the conflicting docstring formats. Much clearer!
jonmaddock
approved these changes
Oct 30, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several updates to the documentation and templates to ensure adherence to the PROCESS style guide and improve overall readability and consistency. The most important changes include adding references to the PROCESS style guide in various files, expanding the style guide documentation, and updating naming conventions in the codebase.
Documentation and Templates Updates:
.github/pull_request_template.md)CONTRIBUTING.md)Style Guide Enhancements:
documentation/proc-pages/development/standards.md)documentation/proc-pages/development/standards.md)documentation/proc-pages/development/standards.md)Navigation Update:
mkdocs.yml)Checklist
I confirm that I have completed the following checks: