Skip to content

Setup bors for Clippy #3348

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

Merged
merged 2 commits into from
Oct 24, 2018
Merged

Setup bors for Clippy #3348

merged 2 commits into from
Oct 24, 2018

Conversation

flip1995
Copy link
Member

Since bors-ng is already installed for this repo for a while (#3279 (comment)), the only thing missing, before we can use it, is the bors.toml. (bors-ng docs)

If we want to move forward with this and this gets merged, the only thing left to do is to create the branches staging and trying.

@phansch @oli-obk

@flip1995 flip1995 added the S-needs-discussion Status: Needs further discussion before merging or work can be started label Oct 22, 2018
@@ -9,6 +9,15 @@ os:

sudo: false

branches:
only:
Copy link
Member Author

Choose a reason for hiding this comment

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

This will prevent that PRs created from a branch within this repo will get checked twice by appveyor and travis. (For example #2783)

# This is where pull requests from "bors try" are built.
- trying
# Also build pull requests.
- master
Copy link
Member Author

Choose a reason for hiding this comment

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

As long as this is activated PRs will still get checked by appveyor and travis (once).

@phansch
Copy link
Member

phansch commented Oct 22, 2018

I set up bors on some of my own repos this weekend and this LGTM.

If I understand the docs correctly:

Note that bors reads bors.toml from the pull requests it’s merging, not the one in master, so changes to the file get checked before they land.

then this will trigger a build already if it creates the trying branch by itself (and if GitHub is working properly again)

bors try

edit: Seems like GitHub is still having issues

@flip1995
Copy link
Member Author

I also setup bors for a university project yesterday. And I like it!

Seems like GitHub is still having issues

Yeah ci hasn't started either. I think/hope this is related to the github issues and not the .travis.yml changes.

Oh and I also need to add the branches configuration to appveyor.

bors bot added a commit that referenced this pull request Oct 22, 2018
@phansch
Copy link
Member

phansch commented Oct 22, 2018

bors try

(GitHub is still working through a backlog of webhook deliveries, but might as well give it another try)

@flip1995
Copy link
Member Author

It already triggered 18bf72c

But I think travis and appveyor are a little bit overloaded as well atm.

@bors
Copy link
Contributor

bors bot commented Oct 22, 2018

try

Timed out

@matthiaskrgr
Copy link
Member

Will we force all merges to go through bors?

If something breaks ci (like the appveyor dogfood test recently) will it mean that no PRs will be merged until that is fixed (even it is unrelated to the PRs wanting to be merged)?

But of course it might lead to such breakages be addressed much faster in the future on the other hand..

@flip1995
Copy link
Member Author

Generally I would say yes, every PR should get merged through bors (it's also easier for the reviewers).

On special occasions, such as unrelated build failures, where the fix might take a few days/weeks, merging by hand is an option, while an infra fix is still preferred.

it might lead to such breakages be addressed much faster

That would be a nice side effect!

bors bot added a commit that referenced this pull request Oct 22, 2018
@flip1995
Copy link
Member Author

flip1995 commented Oct 22, 2018

Well the timeout mechanism works #3348 (comment) 😄

@bors

This comment has been minimized.

@bors
Copy link
Contributor

bors bot commented Oct 22, 2018

try

Timed out

@phansch
Copy link
Member

phansch commented Oct 23, 2018

bors try

bors bot added a commit that referenced this pull request Oct 23, 2018
@bors
Copy link
Contributor

bors bot commented Oct 23, 2018

try

Build succeeded

@matthiaskrgr
Copy link
Member

cool 👍

@phansch
Copy link
Member

phansch commented Oct 23, 2018

We can always add the required checks later on. That's not part of the PR anyway, I guess.

@Manishearth @oli-obk Is there anything else that would speak against merging this?

@oli-obk
Copy link
Contributor

oli-obk commented Oct 23, 2018

Is there anything else that would speak against merging this?

Nope. You wanna do the honors?

@phansch
Copy link
Member

phansch commented Oct 24, 2018

🎉

bors r+

@phansch
Copy link
Member

phansch commented Oct 24, 2018

bors ping

@bors
Copy link
Contributor

bors bot commented Oct 24, 2018

pong

@phansch
Copy link
Member

phansch commented Oct 24, 2018

bors r+

(looks like I had to create the staging branch manually first)

bors bot added a commit that referenced this pull request Oct 24, 2018
3348: Setup bors for Clippy r=phansch a=flip1995

Since [bors-ng](https://app.bors.tech/repositories/3993) is already installed for this repo for a while (#3279 (comment)), the only thing missing, before we can use it, is the `bors.toml`. (bors-ng [docs](https://bors.tech/documentation/getting-started/))

If we want to move forward with this and this gets merged, the only thing left to do is to create the branches `staging` and `trying`.

@phansch @oli-obk 

Co-authored-by: flip1995 <[email protected]>
Co-authored-by: Philipp Krones <[email protected]>
@bors
Copy link
Contributor

bors bot commented Oct 24, 2018

Build succeeded

@bors bors bot merged commit 9086730 into rust-lang:master Oct 24, 2018
@flip1995 flip1995 deleted the bors branch October 24, 2018 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-needs-discussion Status: Needs further discussion before merging or work can be started
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants