Fremake documentation updates to create_checkout and scripts within gfdlfremake#761
Fremake documentation updates to create_checkout and scripts within gfdlfremake#761meteorologist15 wants to merge 7 commits intoNOAA-GFDL:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #761 +/- ##
==========================================
- Coverage 84.08% 84.07% -0.02%
==========================================
Files 70 70
Lines 4775 4771 -4
==========================================
- Hits 4015 4011 -4
Misses 760 760
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
meteorologist15
changed the title
| This script automates the creation of 'checkout.sh' scripts used to clone and manage | ||
| source code for climate model builds within the FRE (FMS Runtime Environment) ecosystem. |
There was a problem hiding this comment.
Translates (or parses) information the resolved yaml configuration to generate a checkout.sh script that git clones the model source code.
| 4. Execution & Lifecycle: Optionally executes the generated script immediately and | ||
| handles forced re-creation of existing checkout scripts. | ||
| 5. Directory Management: Automatically sets up appropriate source and temporary | ||
| directory structures based on the target platform configuration. |
There was a problem hiding this comment.
Lines 7-16 could potentially go into this tool file: https://github.com/NOAA-GFDL/fre-cli/blob/main/docs/tools/make.rst
| import subprocess | ||
| import logging | ||
| from typing import Optional | ||
| from typing import Optional, List, Union |
There was a problem hiding this comment.
use list instead of List for type hinting.
Use "|" instead of Union
| """ | ||
| Write the checkout script for a bare-metal build environment. | ||
|
|
||
| This function initializes a checkout object, writes the necessary shell |
There was a problem hiding this comment.
We previously discussed having this: "This function extracts information from resolved/loaded yaml configuration and generates a checkout script to the source directory for the bare-metal build."
But I think the function below is the main function to extract a lot of info. So here, I agree with "This function generates a checkout script in the source directory for a bare-metal build."
| commands to clone repositories into the source directory, and sets | ||
| appropriate file permissions. | ||
|
|
||
| :param model_yaml: Validated FRE YAML object containing compilation metadata |
There was a problem hiding this comment.
Got ahead of myself before in our meeting. Yes, this is an object that is passed to this function. Maybe we can word it something like: :param model_yaml: FRE YAML class object containing parsed and validated yaml dictionary compilation information
| :type model_yaml: yamlfre.freyaml | ||
| :param src_dir: Path to the directory where source code will be checked out | ||
| :type src_dir: str | ||
| :param jobs: Number of parallel jobs for git submodules (as a string) |
There was a problem hiding this comment.
look at Lauren's fre make doc PR for jobs
| :param target: Predefined FRE target(s); options include [prod/debug/repro]-openmp | ||
| :type target: str or list |
There was a problem hiding this comment.
similar comment to platforms with str or list
| :param njobs: Used in the recursive clone; number of submodules to fetch simultaneously (default 4) | ||
| :type njobs: int | ||
| :param execute: Run the created checkout script to check out source code | ||
| :param execute: Run the created checkout script to check out source code immediately |
There was a problem hiding this comment.
I feel like you don't need "immediately"
|
|
||
| .. note:: For a bare-metal build, no_parallel_checkout = True | ||
| For a container build, no_parallel_checkout = False | ||
| .. note:: For a bare-metal build, no_parallel_checkout typically defaults to True |
There was a problem hiding this comment.
For a bare-metal build - the parallel checkout is the default
Non-parallel checkout is the default for container builds
| ## Error checking the targets | ||
| for target_name in tlist: | ||
| # Validate the targets | ||
| targets_list = [target] if isinstance(target, str) else target |
There was a problem hiding this comment.
From my comment about multiple platforms and targets that can be passed above, I don't think this is necessary since if multiple targets are passed via click options, they're read as a tuple ('platform1', 'platform2')
| pass | ||
| else: | ||
| raise ValueError (f"{platform_name} does not exist in platforms.yaml") | ||
| platforms_list = [platform] if isinstance(platform, str) else platform |
There was a problem hiding this comment.
similar comment to target list bit on line 146
2 participants
Describe your changes
Additional documentation made to create_checkout_script.py, the tests subdirectory, and the gfdlfremake subdirectory within 'make'
Issue ticket number and link (if applicable)
n/a
Checklist before requesting a review