docs: Simplify PR templates #7753
Conversation
yinggeh
changed the title
| - [ ] Added [test plan](#test-plan) and verified test passes. | ||
| - [ ] Verified that the PR passes existing CI. | ||
| - [ ] Verified copyright is correct on all changed files. | ||
| - [ ] Added _succinct_ git squash message before merging [ref](https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html). | ||
| - [ ] All template sections are filled out. | ||
| - [ ] Optional: Related issues are referenced. |
There was a problem hiding this comment.
I don't think this should be optional. It's useful for cherry picks to know of related PRs, and if the PR closes an issue, we should mark it as closed
There was a problem hiding this comment.
It's optional because most PRs do not related to a GitHub issue.
There was a problem hiding this comment.
when I created this PR, I conflated the word "issue". I agree that only sometimes the PR is related to a Github issue. Can you add a checkbox that says:
| - [ ] Optional: Related issues are referenced. | |
| - [ ] Linked related PRs, if any. | |
| - [ ] Optional: Related issues are referenced. |
There was a problem hiding this comment.
There is already a "Related PRs" section.
There was a problem hiding this comment.
I think "related issue" should be a dedicated section, GitHub should be able to link and close the issue automatically if the specified format is followed: https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
| # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE | ||
| # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
| --> | ||
|
|
There was a problem hiding this comment.
will updating copyright here produce a big blob of text in the PR template? If yes, I suggest we remove it
There was a problem hiding this comment.
Not sure if we can merge without passing all pre-commit checks. cc @rmccorm4
There was a problem hiding this comment.
I would argue this piece of code specifically, does not need a copyright. If you are concerned with the pre-commit check for copyright being not accurate, please link the copyright checking script: https://github.com/triton-inference-server/server/blob/main/qa/common/check_copyright.py. You can see that the Pull Request document is excluded from the check as well as general .md files
There was a problem hiding this comment.
It's the recent PR that added add_copyright.py to the pre-commit hook.
We might want to skip some files and paths like we do in check_copyright.py.
See #7666
| #### What does the PR do? | ||
| <!-- Describe your pull request here. Please read the text below the line, and make sure you follow the checklist.--> | ||
|
|
||
| #### Checklist | ||
| - [ ] PR title reflects the change and is of format `<commit_type>: <Title>` | ||
| - [ ] PR title reflects the change and is of format `<commit_type>: <Title>`. Check the [conventional commit type](https://github.com/angular/angular/blob/22b96b9/CONTRIBUTING.md#type). |
There was a problem hiding this comment.
I would want to have a list of commonly used types ("i.e. feat, fix, doc") and mention that the link points to more types.
What does the PR do?
Internal: remove CI Pipeline ID and commit type sections. Optional GitHub issue.
External: remove commit type section.
Checklist
<commit_type>: <Title>Commit Type:
Check the conventional commit type
box here and add the label to the github PR.
Related PRs:
Where should the reviewer start?
Test plan:
Caveats:
Background
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)