-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add pull request and issue templates #372
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, it's identical to the one for ccpp-physics. Thanks for bringing it over. I want to hear from @gold2718 though. The final version of this PR will be merged together with a bunch of other PRs as part of ufs-weather-model PR ufs-community/ufs-weather-model#572.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if we need to discuss these change requests.
## Documentation: | ||
Does this PR add new capabilities that need to be documented or require modifications to the existing documentation? If so, brief supporting material can be provided here. Contact the CODEOWNERS if your PR requires extensive updates to the documentation. See https://github.com/NCAR/ccpp-doc for Technical Documentation or https://dtcenter.org/community-code/common-community-physics-package-ccpp/documentation for the latest Scientific Documentation. | ||
|
||
## Issue (optional): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer that this is not optional. That is, I find it helps to always have at least one issue that has been entered to describe the work (e.g., bug report, feature request).
@JulieSchramm please address @gold2718's comments Tuesday morning, if possible, we want to merge the PR on that day. Thanks! |
@gold2718 in case @JulieSchramm cannot make the changes you requested, is it ok to merge this as is? My PR #366 is next in the ufs-weather-model commit queue, I could make the changes there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Great, can you merge it? Merging is blocked for me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved
Will merge when regression testing is done and Steve approved it (hopefully)
… On May 18, 2021, at 8:58 AM, Laurie Carson ***@***.***> wrote:
@llpcarson approved this pull request.
approved
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub <#372 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2RNHC2OKIDQ6LE5N7I3TOJ6AFANCNFSM442YGR2A>.
|
@gold2718 I am merging this now, any changes you may have I will make in my next PR. Thanks! |
I was in meetings all morning. I am okay with the changes. |
Thanks! |
…pp-physics (#300) - renames the ccpp-framework and ccpp-physics branches from master to main in .gitmodules - updates the submodule pointer for ccpp-physics for several small changes described in NCAR/ccpp-physics#658 - updates the submodule pointer for ccpp-framework for a small update described in NCAR/ccpp-framework#372 - add missing Intel DEBUG flags specific to CCPP
Addresses issue #367
Associated PRs
#372
NCAR/ccpp-physics#658
NOAA-EMC/fv3atm#300
ufs-community/ufs-weather-model#572
Testing
For regression testing, see ufs-community/ufs-weather-model#572