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.

Sign up for GitHub

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

Jump to bottom

chore: improve PR template text #3003

Merged
merged 3 commits into from
Oct 15, 2021
Merged

chore: improve PR template text #3003

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2500f27
chore: improve PR template text
nbbeeken Oct 14, 2021
b5200b4
fix: apply suggestions from code review
nbbeeken Oct 14, 2021
c86bb85
drop bug specific headers, add lint task reminder
nbbeeken Oct 15, 2021
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
30 changes: 27 additions & 3 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,29 @@
## Description
### Description

**What changed?**
#### What is changing?

**Are there any files to ignore?**
##### Is there new documentation needed for these changes?

#### Why does this need changes? Bug? or Feature?

##### 🙁 Actual behavior
Copy link
Contributor

Choose a reason for hiding this comment

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

is it worth noting that this section and the next only apply to bug fix PRs?

For feature PRs it might be worth asking about like the use case for the feature or something like that, to help us evaluate the value of it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into what I mentioned during our meeting about a button that differentiates the text based on a type. And that appears to be for Issues and not PRs. Maybe we drop these to headers. and just stick with #### What is the motivation for this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's reasonable, I think we could also put in the commented text something along the lines of "if filing a bug, please describe the current/desired behavior"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Published changes after I commented so now it has comments like so:

#### What is the motivation for this change?

<!-- If this is a bug, it helps to describe the current behavior and a clear outline of the expected behavior -->
<!-- If this is a feature, it helps to describe the new use case enabled by this change -->


<!-- What happened, and why it was wrong -->

##### 🙂 Expected behavior

<!-- What you expected to happen instead, and why -->

<!--
Contributors!
First of all, thank you so much!!
If you haven't already, it would greatly help the team review this work in a timely manner if you create a JIRA ticket to track this PR.
You can do that here: https://jira.mongodb.org/projects/NODE
-->

### Double check the following

- [ ] Completed self-review using the [steps outlined here](https://github.com/mongodb/node-mongodb-native/blob/HEAD/CONTRIBUTING.md#reviewer-guidelines)
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
- [ ] PR title follows the correct format: `<type>(NODE-xxxx)<!>: <description>`
- [ ] Changes sufficiently covered by new or existing tests
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
- [ ] New TODOs have a related JIRA ticket