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

Update README.md with information of the model configuration file #114

Merged
merged 6 commits into from
Mar 3, 2025

Conversation

jo-basevi
Copy link
Collaborator

@jo-basevi jo-basevi commented Feb 24, 2025

Added description of config/ci.json with some information from https://github.com/ACCESS-NRI/model-configs-template/blob/main/README-DEV.md#ci-configuration. I think it makes the most sense to put this description in this repository so it can be easily updated and have links to this section from the ACCESS-NRI/model-configs-template repository.

I'll leave this draft for now as it references open PRs #113 and #111 for adding more flexibility to the configuration file.

Closes #107

@jo-basevi jo-basevi self-assigned this Feb 24, 2025
@CodeGat CodeGat self-requested a review February 25, 2025 00:00
@jo-basevi
Copy link
Collaborator Author

Related PRs are now merged, so moving this out of draft

@jo-basevi jo-basevi marked this pull request as ready for review February 25, 2025 04:32
Copy link
Member

@CodeGat CodeGat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one thing!

Copy link
Member

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, I just have some suggestions to rearrange a few bits for clarity, but if you don't think they help feel free to push back.

@jo-basevi
Copy link
Collaborator Author

Thanks for the reviews and suggestions! Have added them in

aidanheerdegen
aidanheerdegen previously approved these changes Feb 26, 2025
Copy link
Member

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

anton-seaice
anton-seaice previously approved these changes Feb 26, 2025
Copy link
Contributor

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @jo-basevi - many wording suggestions, some more useful than others

Update README.md with review suggestions

Co-authored-by: Anton Steketee <[email protected]>
@jo-basevi
Copy link
Collaborator Author

Thanks for all the suggestions! The scheduled section/s might need to be updated depending on this issue #118 but that also can be handled in a separate PR.

anton-seaice
anton-seaice previously approved these changes Feb 28, 2025
Copy link
Contributor

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jo-basevi

Co-authored-by: Anton Steketee <[email protected]>
@jo-basevi jo-basevi merged commit c226d84 into main Mar 3, 2025
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.

Add documentation for config/ci.json file
4 participants