Updated PR template #3619
Updated PR template #3619
Conversation
Signed-off-by: szalpal <mszolucha@nvidia.com>
| ### Affected modules and functionalities: | ||
| <!--- Describe here what was changed, added, removed. ---> |
There was a problem hiding this comment.
I think these two points should remain below the first section with the checkboxes.
What happened in this PR
< description goes here >
Category
- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)
- Breaking change (fix or feature that would cause existing functionality to not work as expected)
- Refactoring (Redesign of existing code that doesn't affect functionality)
- Other (e.g. Documentation, Tests, Configuration)
Additional information
Affected modules and functionalities:
Key points relevant for the review:
Checklist
...
Signed-off-by: szalpal <mszolucha@nvidia.com>
.github/PULL_REQUEST_TEMPLATE.md
Outdated
|
|
||
| ### What happened in this PR | ||
| ## This PR is a: |
There was a problem hiding this comment.
Might be personal preference, but I like Category better. "This PR is a " is a bit misleading to me.
There was a problem hiding this comment.
I wanted to fit in the general feeling in this template - This PR is a: fits more with What happened in this PR. If we'd use Category, we'd be better with Description instead of What happened in this PR
There was a problem hiding this comment.
I would prefer a checklist as well.
There was a problem hiding this comment.
It reads a bit weird in my opinion. That's why I think "Category" works better
There was a problem hiding this comment.
BTW, I think that "Description" would be great for the second part.
There was a problem hiding this comment.
Went for Category and Description.
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| <!--- Please pick one from below. Just uncomment what fits most. ---> | ||
| <!--- **Bug fix** (*non-breaking change which fixes an issue*) ---> |
There was a problem hiding this comment.
Please no.
First, it does not say how to uncomment.
Second, to uncomment I need to remove manually both ends of the comment marker. It seems like it will be cumbersome on the day to day basis.
There was a problem hiding this comment.
+1 on uncommenting being cumbersome - copy-paste would be easier.
There was a problem hiding this comment.
I would assume, that if you are contributing to DALI, you do know how to uncomment this piece of code ;)
And for the latter - I believe that comparing to the effort we should put in the PR descriptions (looking into one of @hugo213 's PRs: #3604... these are really impressing), removing 6 characters is really nothing.
There was a problem hiding this comment.
Maybe instead of saying "uncomment" just leave the "Please pick one from below" and then one can copy-paste or uncomment depending on what's easier
Signed-off-by: szalpal <mszolucha@nvidia.com>
* Moving categories around Signed-off-by: szalpal <mszolucha@nvidia.com>
* Moving categories around Signed-off-by: szalpal <mszolucha@nvidia.com>
* Moving categories around Signed-off-by: szalpal <mszolucha@nvidia.com>
Signed-off-by: szalpal mszolucha@nvidia.com
Category:
Other (e.g. Documentation, Tests, Configuration)
Description:
This PR updates the PR template. The main point is that the "What happened in this PR" section will be on top, so we instantly see the details about the PR. On the other hand, "Description" section belongs more to a "Checklist" (since we check things there), thus it's moved there.
Additional information:
Affected modules and functionalities:
.github
Key points relevant for the review:
To see, how the new PR template looks, please go to the rendered file.
Checklist
Tests
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A