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 bors #3732

Closed
wants to merge 5 commits into from
Closed

Add bors #3732

wants to merge 5 commits into from

Conversation

Hoverbear
Copy link
Contributor

What have you changed? (mandatory)

Enables @bors for TiKV.

Learn about bors, or see the CONTRIBUTING.md changes in this PR.

What is Bors?: Bors is a bot who will help us merge PRs faster and reduce CI wait times!
How?: Once a PR has two approvals you will be able to do bors r+ as a maintainer of TiKV to have bors go ahead and try to merge the PR. Bors will automatically merge if there are no conflicts, and if a conflict breaks the build it will report it. For small changes, bors supports a bors rollup command to have it try multiple PRs at once, we have not tried this though. Bors also supports setting priorities on the queue, so important builds can be prioritized.
Why?: Right now our team waits a long time for CI builds, and if another PR is merged all others must rebuild. This is time consuming and slow. Bors will do this for us automagically in a queue.
When?: I will need some time to figure out how to work it with CircleCI and Jenkins. Right now we have it working for AppVeyor and Travis-CI. If you'd like, I am very happy to set up bors on any libraries you are working on as well, but a large project gets the most benefit.

This introduces bors in an optional, supplementary manner.

PRs do not yet require bors to be used to be merged. This is intended for the future work, that PR will impact your workflow!!!

This is a best effort PR, as we may need to tinker with the CI to get bors properly detecting the statuses of CircleCI and (later) Jenkins. Expect follow ups.

What are the type of the changes? (mandatory)

New tooling.

How has this PR been tested? (mandatory)

It adds CI infrastructure, so cannot be practically tested. However if this fails it will not impact our contributors or workflow.

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

Only contributing docs, only for reviewers.

Does this PR affect tidb-ansible update? (mandatory)

Nope

Refer to a related PR or issue link (optional)

tikv/raft-rs#137
tikv/raft-rs#134

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

Signed-off-by: Hoverbear <operator@hoverbear.org>
Signed-off-by: Hoverbear <operator@hoverbear.org>
Signed-off-by: Hoverbear <operator@hoverbear.org>
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
Signed-off-by: Hoverbear <operator@hoverbear.org>
@Hoverbear
Copy link
Contributor Author

This is blocked by #3623.

@Hoverbear
Copy link
Contributor Author

@BusyJay noted that bors currently doesn't squash PRs on merge. :(

@Connor1996 Connor1996 added component/build Component: Build, Deployment, etc. and removed S: WIP labels Nov 19, 2018
@siddontang
Copy link
Contributor

@Hoverbear

noted that bors currently doesn't squash PRs on merge

Seem we can't use this feature now? Or we must acquire that the PR only contains one commit, so the users must square the PR by themselves.

@Hoverbear
Copy link
Contributor Author

@siddontang If we must have squashed merges then bors no longer provides us any benefit (since then they would need to squash their commits after every small feedback, it would make reviewing very hard).

If we can accept non-squashed merges then we can totally use bors and it can still help us. In this case we can encourage our developers to avoid having lots and lots of change or small fix commits before the initial review. So it might encourage us to use better commit style.

What do you think?

@Hoverbear Hoverbear closed this Jul 30, 2019
@siddontang siddontang deleted the bors branch July 30, 2019 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/build Component: Build, Deployment, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants