Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sig-transaction's reviewers #544

Merged
merged 5 commits into from
Sep 9, 2021

Conversation

ekexium
Copy link
Contributor

@ekexium ekexium commented Sep 6, 2021

Add reviewers from the sig-transaction to the TiDB team: @ekexium, @longfangsong, @you06, which is missing in #522

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Sep 6, 2021

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • jackysp

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.

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

@CLAassistant
Copy link

CLAassistant commented Sep 6, 2021

CLA assistant check
All committers have signed the CLA.

@ekexium
Copy link
Contributor Author

ekexium commented Sep 6, 2021

/cc @tisonkun

@ekexium ekexium changed the title add sig-transaction's reviewers Add sig-transaction's reviewers Sep 6, 2021
@tisonkun
Copy link
Contributor

tisonkun commented Sep 6, 2021

@ekexium thanks for creating this PR. I'd suggest you create a vote entry under the directory https://github.com/pingcap/community/tree/master/votes and elaborate nominees' contribution (of course, previous sig-transaction's reviewers is enough, but better to have more details on their work yet).

See also the template and https://github.com/tikv/community/pull/139/files#diff-2501407e9d546f432e26d56687cdf445fd1b9798b541cbeeeaee491b00a44fd3

Signed-off-by: ekexium <[email protected]>
teams/tidb/membership.json Outdated Show resolved Hide resolved
@you06
Copy link
Contributor

you06 commented Sep 7, 2021

We just move the reviewers from tikv/community to here, and we three were promoted as reviewers in that place. Do we need to create vote entry here?

https://github.com/tikv/community/blob/6e036b68e1d9f27b264895512219dd0c0804f68a/sig/transaction/membership.json#L53-L63

@tisonkun
Copy link
Contributor

tisonkun commented Sep 7, 2021

@you06 actually I call for the migration in #522 multiple times but there is no suggestion on this memberships.

It's ok to have some information catch up later but missing the migration window I'd prefer you guys join the team via normal process, i.e., a vote.

As mentioned here tikv/community#142 (comment), we tend to avoid exceptions. And a nomination for reviewers, with approvals, last for 72 hours. It won't take a long time.

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

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

cfzjywxk commented Sep 7, 2021

LGTM

Copy link
Contributor

@MyonKeminta MyonKeminta left a comment

Choose a reason for hiding this comment

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

I didn't understand it well. How can I vote? Is an approval to this PR enough?

@ti-chi-bot
Copy link
Member

@MyonKeminta: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

I didn't understand it well. How can I vote? Is an approval to this PR enough?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@tisonkun
Copy link
Contributor

tisonkun commented Sep 8, 2021

@MyonKeminta thanks for showing you approval. According the governance rules accepted earlier #522

Decisions regarding the project are made by votes on the community repository. Votes are clearly indicated by a pull request adding an entry under votes folder. Votes may contain multiple items for approval and these should be clearly separated. Voting is carried out by replying to the vote pull request. Voting may take three flavors

You can show your approval by GitHub out-of-the-box approval.

For team decisions, only the votes of active team maintainers are binding. Non-binding votes are still useful for those with binding votes to understand the perception of an action in the wider community.

So only @pingcap/tidb-maintainers has binding votes to this decision process. Only binding vote counts.

For promote a new reviewer, we require a minimal length on 3 days so with 2 approval from @jackysp & @cfzjywxk , this votes will be closed as accepted tomorrow if there is no further objections.

@cfzjywxk
Copy link
Contributor

cfzjywxk commented Sep 9, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 09fc57e

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 9, 2021
@tisonkun
Copy link
Contributor

tisonkun commented Sep 9, 2021

@cfzjywxk I think you should sign CLA for this repo 😂

@ti-chi-bot ti-chi-bot merged commit 3ba3dde into pingcap:master Sep 9, 2021
@tisonkun
Copy link
Contributor

tisonkun commented Sep 9, 2021

@ekexium @you06 @longfangsong I've added you in the tidb-reviewers team.

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.

8 participants