Change the method of PR associated issue
Mini256 opened this issue · 13 comments
In voting-150, we passed the scheme of using commit message in the PR to associate the issue, so that the squash commit can also associate with the issue.
But in the process of practice, it will bring some troubles to code contributors, especially in the case where we have multiple release branches, we need to submit meaningless commits or modify the commit message many times to pass the check.
For now, we forked out a customized version of prow, which is the upstream code repository of the @ti-chi-bot .
Based on it, we add some functions to make @ti-chi-bot be able to automatically extract issue numbers from the PR body and automatically populate the issue number to the squash commit message when merging.
For example:
We just need to fill in a line starting with Issue Number: of text in the PR body. (For concrete rules, please refer to the description in the TiDB development guide.)
The robot will extract the issue number section in the squash commit message when merging.
I think that this schema replacing our original schema will make our work easier. If that makes sense to everyone, I will
-
Disable the format-checker on tikv and pd repo, no longer enforcement requires issue number in PR's commmit.
-
Call for a public vote on the new schema, continue to complete the issue #149 ( cc: @zhangyangyu )
What if the author has written the issue number in the commit message already?
What if the author has written the issue number in the commit message already?
@BusyJay I think it's OK, because in the previous proposal, we also suggested that PR authors should associate issues in the PR body, the vast majority of PR meet the requirement of the new scheme and the old scheme at the same time.
- A pull request SHOULD be with a description mention the issue(s) associated, so that reviewers are able to get the information directly.
But I found that our PR template has changed, which causes subsequent new PRs that cannot meet the format requirements in the new schema.
I suggest that we first add the prefix Issue Number: to the PR template again, which can make the template clearer and make text matching easily by the robot. After a period of transition, we can require the format of the issue number to be correct.
...I think it's OK...
What I mean is if author has written the issue number in commit message already, will the bot append the issue number again to the squashed commit message?
...I think it's OK...
What I mean is if author has written the issue number in commit message already, will the bot append the issue number again to the squashed commit message?
I think yes. The new method won't check any commit message and just append the issue number in the PR body to the final squashed commit message. We put the restriction that a PR must have a relevant issue from commit message to PR body.
Got it. It's a nice to have to skip appending if the issue is mentioned in the commit message. The overall idea lgtm.
will the bot append the issue number again to the squashed commit message?
The bot currently only extracts the issue number from the specified line of the PR body to the squashed commit message, but for the entire message of squashed commit, this depends on how we handle the message of commits in the PR.
- If we need to list each commit according to the requirements of DCO, the entire message of squashed commit will be:
the title of pull request
close #{{issue_numer_in_the_pr_body}}
* the message title of commit 1 in the PR
Signed-off-by: xxx <xxx@example.com>
* the message title of commit 2 in the PR
close #{{issue_numer_in_the_commit_message}}
Signed-off-by: xxx <xxx@example.com>
Both issue numbers in the PR body and commits will be appended to the squashed commit message.
- If we don't need to list each commit, the entire message of the squashed commit will be:
the title of the pull request
close #{{issue_numer_in_the_pr_body}}
Signed-off-by: xxx <xxx@example.com>
If DCO does not have such a strict requirement, I am willing to implement the second way through the bot, but I am not clear about the requirement of DCO.
If DCO does not have such a strict requirement, I am willing to implement the second way through the bot, but I am not clear about the requirement of DCO.
I asked experts who knew this better, it seems that CNCF does not require the commit message after the merging. And I found the other CNCF project (fox example: envoyproxy/envoy) can only remain one line Signed-off-by, so I'll start implementing the second format.
I suggest to keep the commit messages or the PR body (only the content from "What's Change").
On the other hand, signoff lines should match all author fields.
If DCO does not have such a strict requirement, I am willing to implement the second way through the bot, but I am not clear about the requirement of DCO.
I asked experts who knew this better, it seems that CNCF does not require the commit message after the merging. And I found the other CNCF project (fox example: envoyproxy/envoy) can only remain one line
Signed-off-by, so I'll start implementing the second format.
It's great that we can format commit message body whatever the way we like. As the formatting logic is becoming more powerful, we can really make it informative yet not too verbose.
I suggest to keep the commit messages or the PR body (only the content from "What's Change").
If we retain the commit message in the PR, the problem with squash commit message redundancy is not resolved.
I think it would be a better idea for the bot to extract the content from "What's Change" part of the PR body.
Maybe we can define a code block like release-note to tell the bot to extract this part to the squashed commit message, for example:
### What is changed and how it works?
Issue Number: close #xxx
What's Changed:
```commit-message
Describe what PR has changed, and the content in the code block will be extracted into the squared commit message
```
closed by ti-community-infra/configs#505