-
Notifications
You must be signed in to change notification settings - Fork 444
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
918d4a0
commit 2b4ac35
Showing
2 changed files
with
8 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,3 +26,7 @@ env: | |
notifications: | ||
email: | ||
on_success: never | ||
branches: | ||
only: | ||
- master | ||
- auto |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2b4ac35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this mean you are running tests twice? Once when the PR gets pushed to
auto
by bors, and then again when bors moves it to master?AFAIK the usual setup is to only run on
auto
andtry
. You only need master CI if you want it to run in PRs (and then it should probably condition on really being in a PR).2b4ac35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I stopped using bors a long time ago because I found it to be a pain in the ass for a low traffic repo like
regex
. I was using bors though, so theauto
branch here is vestigial.2b4ac35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, thanks. :)
AppVeyor still runs twice, once for PRs and once when it lands in master, but for low-traffic repos that won't contribute much to our overall AppVeyor load. (All rust-lang-libs projects share a single AppVeyor runner, and that can be quite congested these days.)
2b4ac35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that is intentional. Travis should be doing the same thing. That ensures both the PR and the result of merging the PR are checked in CI. bors is better here because it checks the result of the merge before actually merging it.
(The other reason I don't like bors is because it, AIUI, forces you to use merge commits and I don't like merge commits.)
2b4ac35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current bors does, yes, though that doesn't seem fundamental to the approach.
I quite like the merge commits actually, but YMMV (and clearly does). ;)
There's still regex bors config at https://github.com/rust-lang/rust-central-station/blob/b883b4799fbaf83765fb5c10d16be1f8b529b0aa/homu.toml.template#L201 btw, might be better to remove it then?
2b4ac35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had no idea that repo existed. (Looks like it didn't back when I was using bors. :P) In any case, I submitted a PR: rust-lang/rust-central-station#280