Skip to content

vote for pr issue association rule#606

Merged
ti-chi-bot merged 1 commit intopingcap:masterfrom
zhangyangyu:assoc-pr-issue
Dec 7, 2021
Merged

vote for pr issue association rule#606
ti-chi-bot merged 1 commit intopingcap:masterfrom
zhangyangyu:assoc-pr-issue

Conversation

@zhangyangyu
Copy link
Member

@zhangyangyu zhangyangyu commented Nov 29, 2021

ref #604

Co-authored-by: @tisonkun
Co-authored-by: @Mini256

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Nov 29, 2021

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • sunxiaoguang

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@zhangyangyu
Copy link
Member Author

/hold
/cc @pingcap/tidb-maintainers @pingcap/migration-maintainers

@ti-chi-bot ti-chi-bot requested review from a team November 29, 2021 09:23
@ti-chi-bot ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 29, 2021
@zhangyangyu
Copy link
Member Author

also /cc @pingcap/tidb-committers @pingcap/tidb-reviewers since this affects everyone


Issue number in PR body will become a mandatory requirement. Currently, the PR template contains one line `Issue Number: close #xxx`, but this is not mandatory. Contributors are free to leave it empty or delete the whole content.

After this proposal, the bot will check whether there is relevant issue number(s) in the PR body, by `Issue Number: ref #{issue-number}` or `Issue Number: close[sd]?|resolve[sd]?|fix(e[sd])? #{issue-number}`. `ref` means associated with an issue but does not close it. There should be no other content in the same line and the check is case-insensitive. Multiple issues should be separated by a comma, like `Issue Number: close #12341, close #12342`. For PRs trying to close issues in a different repository, contributors need to first create an issue in the same repository and use this issue to track.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we tolerate “close https://github.com/pingcap/repo/issue/1234”? sometimes i like copy and paste urls and this also enables cross repo reference

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 this suppose to work as long as it points to a valid issue link.

Copy link
Member Author

@zhangyangyu zhangyangyu Nov 29, 2021

Choose a reason for hiding this comment

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

Actually we follow Github's rule https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword with some tweaks. First add ref and then disallow cross repo reference. ref adds one semantic. And forcing the issue and PR in one repo helps other members like QA and release managers' work(they may need to apply labels to the issues). We recommend create a new issue in the repo and use the issue to track.

Copy link
Contributor

Choose a reason for hiding this comment

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

it helps other members like QA and release managers' work(they may need to apply labels to the issues)

if they need add labels, less issue means less manual work. if one problem is related to many repo (like need to update dependency) more redundant issues will be spawned.

Copy link
Member Author

@zhangyangyu zhangyangyu Nov 30, 2021

Choose a reason for hiding this comment

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

It makes some sense. But this means you have to take all related repos into considerations as a whole and switching back and forth to track the issue. If the repos don't share the same version strategy it also makes life harder. Updating dependencies sometimes means a bugfix so may affects multiple version so a dedicated issue could help. Such scenarios won't occur frequently.

Copy link
Contributor

Choose a reason for hiding this comment

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

Talking about repos, we are consolidating repos. It's going to help reducing redundant issues.

@zhangyangyu zhangyangyu mentioned this pull request Dec 3, 2021
5 tasks
@zhangyangyu
Copy link
Member Author

ping @pingcap/tidb-maintainers @pingcap/migration-maintainers

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 7, 2021
@sunxiaoguang
Copy link
Contributor

/unhold

@ti-chi-bot ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 7, 2021
@sunxiaoguang
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

DetailsCommit hash: ce8516c

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 7, 2021
@ti-chi-bot ti-chi-bot merged commit fefd905 into pingcap:master Dec 7, 2021
@zhangyangyu zhangyangyu deleted the assoc-pr-issue branch December 7, 2021 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants