Lfric macros jules meta#230
Lfric macros jules meta#230James Bruten (james-bruten-mo) wants to merge 7 commits intoMetOffice:mainfrom
Conversation
Pierre Siddall (Pierre-siddall)
left a comment
There was a problem hiding this comment.
Thanks James Bruten (@james-bruten-mo), semantically this looks really good, I just have a couple of trivial edits with arguments to help clarify information that is being passed via a function or the command line. Let me know if there's anything I can help clarify.
| # Use /tmp for Core and Jules as these are not required for testing | ||
| applymacros = ApplyMacros("vn0.0_t001", None, None, TEST_APPS_DIR, Path("/tmp"), True) | ||
| applymacros = ApplyMacros( | ||
| "vn0.0_t001", None, None, TEST_APPS_DIR, Path("/tmp"), Path("/tmp"), True |
There was a problem hiding this comment.
I reckon it would be good to differentiate between the parameters here as it makes it clearer what data is being operated on when the function is called.
| "vn0.0_t001", None, None, TEST_APPS_DIR, Path("/tmp"), Path("/tmp"), True | |
| "vn0.0_t001", None, None, apps=TEST_APPS_DIR, core=Path("/tmp"), jules=Path("/tmp"), testing=True |
| ) | ||
| parser.add_argument( | ||
| "-j", | ||
| "--jules", |
There was a problem hiding this comment.
It could be easy to get a bit confused with apps and core being references to paths. So adding version here may help users of the script know what is expected in the argument.
| "--jules", | |
| "--jules_version", |
James Bruten (james-bruten-mo)
left a comment
There was a problem hiding this comment.
Thanks Pierre, good suggestions. Both applied
Pierre Siddall (Pierre-siddall)
left a comment
There was a problem hiding this comment.
Thanks James, this looks good to head into code review to me now.
PR Summary
Sci/Tech Reviewer: Pierre Siddall (@Pierre-siddall)
Code Reviewer: Sam Clarke-Green (@t00sa)
This makes changes to the apply_macros and release_new_version scripts to account for lfric-jules metadata now being in the Jules repository.
Code Quality Checklist
Testing
Security Considerations
AI Assistance and Attribution
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review