Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ Developers listed in the CODEOWNERS file will automatically be assigned to revie
If your contribution should be reviewed by anyone else, assign those reviewers manually from the
menu at right, or by tagging them with @USERNAME in the Description text.

Changes to standard names and/or descriptions should be made in the standard_names.xml file. The
files Metadata-standard-names.md and Metadata-standard-names.yaml will need to be updated before
a pull request is merged; this can be done manually by the PR author using the script
tools/write_standard_name_table.py or will be done by the Code Manager prior to merging via Github
Actions.

Be sure to check in on the PR regularly to respond to comments/questions/reviews!
-->

Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/pull_request_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ jobs:

- name: Test rendering xml file to markdown
run: |
# Checks if the saved markdown matches freshly rendered markdown.
# If this fails, prompt user to update
checksum=$(sha256sum Metadata-standard-names.md)
tools/write_standard_name_table.py --output-format md standard_names.xml
echo "The following changes will be committed when this pull request is merged (git diff Metadata-standard-names.md; "
echo "assuming that 'Metadata-standard-names.md' wasn't updated and matches the version in the authoritative branch):"
git diff Metadata-standard-names.md
test "$checksum" = "$(sha256sum Metadata-standard-names.md)" || exit "Markdown file Metadata-standard-names.md must be updated; see documentation for details"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just run git diff after line 78 and check the return code?

Generally, why do we need this. I would revert the changes to pull_request_ci.yml entirely. Can't we just add a checkbox to the pull request template that instructs the code managers to check if the rendered files need to be updated, run the workflow_dispatch action and then tick the box? See #123

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm happy removing these actions all together and returning to the manual updates of old. It would be nice to have an automatic (or at least push-button) solution to update these tables within GitHub but it could be getting that working seems to be more trouble than it's worth.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder, yes. Maybe the best approach is a pragmatic one. Your PR has it so that the checks fail because the files must be updated. If we add a big bold message there (and in the documentation) that tells the user to update and commit the files, and how to do it, then that is perfectly fine for me.


- name: Test rendering xml file to yaml
run: |
checksum=$(sha256sum Metadata-standard-names.yaml)
tools/write_standard_name_table.py --output-format yaml standard_names.xml
echo "The following changes will be committed when this pull request is merged (git diff Metadata-standard-names.yaml; "
echo "assuming that 'Metadata-standard-names.yaml' wasn't updated and matches the version in the authoritative branch):"
git diff Metadata-standard-names.yaml
test "$checksum" = "$(sha256sum Metadata-standard-names.yaml)" || exit "YAML file Metadata-standard-names.yaml must be updated; see documentation for details"
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
name: Render xml standard name dictionary to markdown and yaml and commit to repository

on:
push:
branches:
- main
workflow_dispatch:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know if this works? How does this change circumvent the branch protection that the original logic doesn't?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the commit is done on the PR branch prior to merge rather than in main then there are no branch protection issues (I think). But this would require that the user's fork has been updated to include this workflow file already, since workflow_dispatch tasks must be in that specific repository's main branch.


jobs:
update-md-and-yaml:
Expand Down
2 changes: 1 addition & 1 deletion Metadata-standard-names.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Earth System Modeling Standard Name Library - add some chunk here to test auto update
# Earth System Modeling Standard Name Library
#### Table of Contents
* [dimensions](#dimensions)
* [constants](#constants)
Expand Down
2 changes: 1 addition & 1 deletion Metadata-standard-names.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
library_name: Earth System Modeling Standard Name Library - add some chunk here to test auto update
library_name: Earth System Modeling Standard Name Library
sections:
- name: dimensions
comment: 'Dimension standard names may come in sets of six related standard names
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ The Earth System Modeling Standard Names Repository contains community-accepted

Rules governing the designation and format of standard names can be found in [StandardNamesRules.rst](https://github.com/ESCOMP/ESMStandardNames/blob/main/StandardNamesRules.rst)

A [Markdown file describing the standard names is included](https://github.com/ESCOMP/ESMStandardNames/blob/main/Metadata-standard-names.md), as well as a [Yaml version of the XML file](https://github.com/ESCOMP/ESMStandardNames/blob/main/Metadata-standard-names.yaml).
A [Markdown file describing the standard names is included](https://github.com/ESCOMP/ESMStandardNames/blob/main/Metadata-standard-names.md), as well as a [YAML version of the XML file](https://github.com/ESCOMP/ESMStandardNames/blob/main/Metadata-standard-names.yaml).

Edits to standard names must be made in the xml file `standard_names.xml` only. When pull requests are merged into the authoritative branch, a tool is run in GitHub actions that automatically updates the human-readable standard name Markdown file and the Yaml version.
Edits to standard names must be made in the xml file `standard_names.xml` only. When a pull request is opened into the main branch, the YAML and Markdown files should be updated using the `tools/write_standard_name_table.py` script. This can be done manually by the pull request author, or by activating the GitHub action available on an open pull request.