Replies: 3 comments 1 reply
-
|
@billsacks I don't object; it sounds more impactful perhaps on the CESM side than for UFS (which boils down to either Ufuk or myself). |
Beta Was this translation helpful? Give feedback.
-
|
@billsacks I think this is great idea. BUT, I think the responsibilities of the code owners need to be defined explicitly (or precisely). I did some CMEPS related work in the past but I am hesitant to be a code owner. If it is just a PR review thing than that is fine and I could do my best (still some places deep in the CMEPS is fuzzy to me) but we need to draw the boundary very carefully in here. I would not be in a situation that somebody say that you are the code owner and make this change. Plus my response time for PRs is also related with my other commitments/projects. Anyway, I am open to more discussion about it. |
Beta Was this translation helpful? Give feedback.
-
|
I'm moving ahead with this. Some details: For the branch protection rule: It looks like the only special branch currently is main. I will restrict pushing to this branch to: @jedwards4b , @mvertens , @DeniseWorthen , @uturuncoglu , @fischer-ncar (this addition of Chris is additional to my previous list because I realized that Chris sometimes brings in CMEPS changes needed for CESM) and myself. Based on feedback from a few people here, I will give write access to @megandevlan , and add her in a CODEOWNERS file as an owner of the atm-ocn flux code. I'll open a PR for this soon. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
@jedwards4b @briandobbins @mvertens @DeniseWorthen @uturuncoglu and anyone else interested - I'd like to hear feedback from you on some tentative plans I have to distribute some of the responsibility for CMEPS maintenance to appropriate people within CESM. This started from a discussion yesterday where some people here expressed a desire to be kept more in the loop about changes made to the atm-ocn flux code, but has evolved in my head to dovetail with a desire I've had to distribute the responsibility for CMEPS reviews and maintenance among more people in CESM.
I'd like to introduce a CODEOWNERS file that can be used to automatically assign reviewers for particular directories (or even files). To start, I just imagine using this for the atm-ocn flux calculations, to automatically assign one or two scientists here as reviewers when that code is touched - to keep them in the loop and give them an opportunity to review the code if they want. I hope to avoid having this add significant friction, so I wouldn't want this to be a blocking review – so, for example, if the changes are pretty trivial, I think it would be reasonable to remove their review request and move ahead even without their review, but at least this would lead to them getting a notification for the change and an opportunity to review it if they want.
To make this possible, we'd need to give repository write access to these people (that's a requirement of adding someone to CODEOWNERS, frustratingly). I feel more comfortable doing that if we first put in place branch protection rules for the main branch and any other "important" branch (e.g., release branches), so that only limited people have permission to modify these special branches.
So, to summarize, I propose doing the following, and would like your feedback / approval on this:
cesm/flux_atmocndirectory. Note that it's also possible to put CODEOWNERS in the.githubdirectory, but I lean towards putting it at the top level for visibility.This could later be extended to add other CODEOWNERS for this or other parts of the code. Note that it's possible to use this mechanism to assign default reviewers for all PRs. I'm not sure if that's something we want or not....
Please let me know if you have any thoughts on this.
Beta Was this translation helpful? Give feedback.
All reactions