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

Variable name changing policy and procedures #58

Closed
svahl991 opened this issue Feb 29, 2024 · 5 comments · Fixed by #66
Closed

Variable name changing policy and procedures #58

svahl991 opened this issue Feb 29, 2024 · 5 comments · Fixed by #66

Comments

@svahl991
Copy link
Collaborator

As part of the adoption of the CCPP Standard Names by the JEDI project, the name cloud_area_fraction_in_atmosphere_layer was added to the standard in #28. Later we discovered that the name had been changed to nonconvective_cloud_area_fraction_in_atmosphere_layer in #37, without tagging the creator of #28 (myself) in the PR.

This issue isn't about the merits of making this particular change, but to ask about name changes in general. Do changes like this happen frequently? As the JEDI code adopts and hard-codes these names, a change like this will take code that was previously compliant with the standard and make it non-compliant. It will often be a development effort across multiple repositories and organizations to adjust to a changed name. And if, rather than hard-coding the names, we must create variables to store the name strings and then use those variables in our code, we lose much of the benefit of using these names.

Hopefully name changes like this are infrequent for these reasons. But if a name change must be made, is there a way for all stakeholders to receive a push notification? We stumbled upon this change by accident, and would not have known that action was required if we wanted our code to remain naming compliant.

@climbfuji
Copy link
Collaborator

@svahl991 Sorry Steve, this also sneaked past me. I think what we can do is start using CODEOWNERS and make their reviews mandatory. It's not going to be feasible to check which change may potentially affect which stakeholder; rather, we'd have every stakeholder notified by default if the dictionaries are updated (changed or new names added) and it would be up to the stakeholders to ignore the review request/state they don't care, or review the PRs?

@svahl991
Copy link
Collaborator Author

svahl991 commented Mar 6, 2024

I notice that right now this repository only has a main branch. Has it ever been considered to have periodic "release" branches of this naming repository? (quarterly or semi-annually, for example) Then JEDI could definitively proclaim compliance with a particular release, and have occasional sprints to update its code to comply with the latest release. When a new release is created, there could be release notes pointing out name changes and additions.

@gold2718
Copy link
Collaborator

gold2718 commented Mar 7, 2024

I notice that right now this repository only has a main branch. Has it ever been considered to have periodic "release" branches of this naming repository? (quarterly or semi-annually, for example) Then JEDI could definitively proclaim compliance with a particular release, and have occasional sprints to update its code to comply with the latest release. When a new release is created, there could be release notes pointing out name changes and additions.

How about tags that correspond to CCPP and/or JEDI releases? Something more descriptive than v0.1.1 would be nice. I think that would be more stable than branches. If a diverging release branch is ever needed, it can always be created from the appropriate tag but I would hope that is a rare occurrence.

@nusbaume
Copy link
Collaborator

nusbaume commented Mar 7, 2024

First off, sorry for the surprise standard name changes! I am pretty sure for PR #37 I just randomly selected folks who had reviewed PRs in the past, which is probably why it slipped through un-noticed. So I completely agree that having a more formal workflow for any PRs that change already existing standard names would be a good idea.

Personally I would probably vote for @climbfuji's suggestion of having a code owners file with at least one representative from each invested lab or project (e.g. NCAR CGD and RAL, NOAA, JEDI/UK Met Office, NorESM). We could then require that each member sign off for any PR that changes an already existing standard name, which I imagine we could mark with a label on Github.

For any other PR (e.g. ones that just add new names or add some sort of tool functionality) we can just require one or two of the codeowners to reduce the review burden and ensure PRs don't end up languishing.

Otherwise I am happy if we want to either tag the repo or have release branches. Part of me leans towards just having tags for now as it sounds like less work, but if other folks prefer release branches then I could probably be convinced.

Anyways, if at least the "required review" idea sounds reasonable to everyone then I can try and add an initial code owners file along with a wiki page (and a link in the README) that fully describes the review workflow.

Thanks!

@svahl991
Copy link
Collaborator Author

My opinion is that both the "required review" idea and the repo tagging idea have merits. The required review ensures all stakeholders get a chance to chime in on changes. And the tagging allows people to reference certain fixed points in the naming standard when that is more useful than the changeable main branch.

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 a pull request may close this issue.

4 participants