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

Update PULL_REQUEST_TEMPLATE.md #1070

Closed
wants to merge 1 commit into from
Closed

Conversation

ckoven
Copy link
Contributor

@ckoven ckoven commented Aug 24, 2023

Slight rewording to PR template revisions

Description:

Collaborators:

Expectation of Answer Changes:

Checklist:

  • My change requires a change to the documentation.
  • I have updated the in-code documentation .AND. (the technical note .OR. the wiki) accordingly.
  • I have read the CONTRIBUTING document.
  • FATES PASS/FAIL regression tests were run
  • If answers were expected to change, evaluation was performed and provided

Test Results:

CTSM (or) E3SM (specify which) test hash-tag:

CTSM (or) E3SM (specify which) baseline hash-tag:

FATES baseline hash-tag:

Test Output:

Slight rewording to PR template revisions
@glemieux
Copy link
Contributor

glemieux commented Aug 24, 2023

I think installing https://github.com/marketplace/task-list-completed to our repo would help give more weight to the checklist. I would install this app after this PR merge. CTSM currently uses this app as well.

Note that it blocks a merge provides the option to block the merge unless all the boxes are checked. If we took advantage of this it would force us to review the checklist.

Thoughts @ckoven and @rgknox?

@ckoven
Copy link
Contributor Author

ckoven commented Aug 24, 2023

Thanks @glemieux. Can that package handle an either/or checkbox case as in my proposed revision here?

@glemieux
Copy link
Contributor

glemieux commented Aug 25, 2023

@ckoven it looks like not yet based on this issue: stilliard/github-task-list-completed#21. Based on that, I think I would modify this PR to add a "Documentation" subsection that we ask people to fill out with either the url or a comment stating no update necessary. Then the checklist button could become a directive to respond to that subsection (which is similar to what we do for the "Testing" subsection now):

### Checklist
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
*If this is your first time contributing, please read the [**CONTRIBUTING**](https://github.com/NGEET/fates/blob/main/CONTRIBUTING.md) document.*

All checklist items must be checked to enable merging this pull request:
- [ ] The in-code documentation has been updated with descriptive comments
- [ ] The documentation has been assessed to determine if updates are necessary
- [ ] FATES PASS/FAIL regression tests were run
- [ ] Evaluation of test results for answer changes was performed and results provided

### Documentation
<!--- If this pull requests warrants an update to the tech doc or user's guide, and said changes have been made paste a link to the documentation pull request below.  ->
<!--- If documentation updates are needed, but changes do not yet have their own separate pull request, please create an issue on either repo so that we can keep track of necessary updates.->
- [Technical Note](https://github.com/NGEET/fates-docs):
- [User's Guide](https://github.com/NGEET/fates-users-guide): 

@glemieux
Copy link
Contributor

@ckoven I updated the original PR with the changes above: https://github.com/NGEET/fates/pull/1069/files

@glemieux glemieux mentioned this pull request Aug 25, 2023
5 tasks
@adrifoster
Copy link
Contributor

FYI here is the CAM template for PRs

https://github.com/ESCOMP/CAM/blob/main/.github/ISSUE_TEMPLATE/enhancement_request.yml

@glemieux
Copy link
Contributor

Thanks @adrifoster

@glemieux
Copy link
Contributor

glemieux commented Aug 29, 2023

FYI here is the CAM template for PRs

https://github.com/ESCOMP/CAM/blob/main/.github/ISSUE_TEMPLATE/enhancement_request.yml

It looks like this is using a beta feature (that's been in beta since 2021) and may only be for issue templates currently. I'll give this a try on my personal repo to confirm.

@adrifoster
Copy link
Contributor

FYI here is the CAM template for PRs
https://github.com/ESCOMP/CAM/blob/main/.github/ISSUE_TEMPLATE/enhancement_request.yml

It looks like this is using a beta feature (that's been in beta since 2021) and may only be for issue templates currently. I'll give this a try on my personal repo to confirm.

Ah darn okay. thanks for checking it out.

@glemieux
Copy link
Contributor

FYI here is the CAM template for PRs
https://github.com/ESCOMP/CAM/blob/main/.github/ISSUE_TEMPLATE/enhancement_request.yml

It looks like this is using a beta feature (that's been in beta since 2021) and may only be for issue templates currently. I'll give this a try on my personal repo to confirm.

Ah darn okay. thanks for checking it out.

I just tested this on an old private repo and can confirm the forms don't seem to work for PRs yet. I did try the issue form version and that worked like a charm though!

@glemieux
Copy link
Contributor

Closing after incorporating 397d822 into #1069.

@glemieux glemieux closed this Aug 30, 2023
@glemieux glemieux deleted the ckoven-patch-1 branch September 18, 2023 22:43
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.

3 participants