-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add two more reminders to PR template #5666
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.
Thanks for getting to this. I don't feel strongly about the labeling reminder and vote for merging.
|
||
## Breaking changes and new features | ||
|
||
- [ ] I have added any needed `breaking-change` and `new-feature` labels for the appropriate packages. |
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.
Should internal improvement
be added here? I don't feel strongly about it, as there are many other labels that can apply, and we should use our judgement.
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.
Yes, I thought about adding not just "internal improvement" but that whole category -- enhancement, bug fix, dependencies -- but I didn't think it was important enough. But if people think it's important enough, that could be added too.
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.
Thanks @haltman-at for doing this.
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 like it
Per @dongmingh's suggestion, I've added a reminder to include testing instructions. Also, I added a reminder to add any needed
breaking-change
ornew-feature
labels.