Skip to content

tweaks for jules-lfric metadata#625

Open
James Bruten (james-bruten-mo) wants to merge 5 commits into
MetOffice:mainfrom
james-bruten-mo:lfric_jules_meta
Open

tweaks for jules-lfric metadata#625
James Bruten (james-bruten-mo) wants to merge 5 commits into
MetOffice:mainfrom
james-bruten-mo:lfric_jules_meta

Conversation

@james-bruten-mo
Copy link
Copy Markdown
Contributor

PR Summary

Code Reviewer: Sam Clarke-Green (@t00sa)

This makes some minor changes surrounding shared jules metadata, linked with MetOffice/lfric_apps#463

Code Quality Checklist

  • I have performed a self-review of my own code
  • I have locally built the documentation successfully, and the output of changed sections is as expected

Code Review

  • The changes are coherent and valid

Copy link
Copy Markdown
Collaborator

@maggiehendry Maggie (maggiehendry) left a comment

Choose a reason for hiding this comment

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

I had a look at my previous PR when I imported jules-shared to the UM. /pull/138

I've identified another place which needs a tweak.

James Bruten (@james-bruten-mo) I didn't make this comment particularly clear what needed changing:
The "Important" box in https://metoffice.github.io/simulation-systems/Development/TestSuites/jules.html reads:

For jules-shared changes, when LFRic testing, the changes need to be manually synced to the LFRic location. When UM testing, this is not required as jules-shared is imported from the JULES branch.

which will need updating as it no longer needs manual syncing.

Most of the shared metadata stuff is described in the wiki which I need to migrate to GitHub. The links to this therefore are currently broken.

Comment thread source/Development/metadata_guidance.rst Outdated
Comment thread source/Development/metadata_guidance.rst
Co-authored-by: Maggie <145924708+maggiehendry@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@maggiehendry Maggie (maggiehendry) left a comment

Choose a reason for hiding this comment

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

Please see the other comment regarding another place that needs tweaking which mentions the need to manually sync jules-shared.

Comment thread source/Development/metadata_guidance.rst Outdated
Co-authored-by: Maggie <145924708+maggiehendry@users.noreply.github.com>
@james-bruten-mo
Copy link
Copy Markdown
Contributor Author

Thanks Maggie, made those further tweaks. I don't think the important box in the JULES testing page is adding much anymore, so I've removed that. Instead I've added a new section to the macros page, advising that a macro in um-atmos and jules-lsm will be required for jules-shared changes. It's slightly duplicating some of the lines in the metadata-guidance section, but worth highlighting under macros.

Copy link
Copy Markdown
Collaborator

@maggiehendry Maggie (maggiehendry) left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes. I think it all looks good now. 😄.

Copy link
Copy Markdown
Collaborator

@t00sa Sam Clarke-Green (t00sa) left a comment

Choose a reason for hiding this comment

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

Changes look sensible and render correctly. PR approved and will be committed once other changes are ready to go.

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.

4 participants