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

Deprioritize builds that are in the "try" branch. #4

Merged
merged 1 commit into from Dec 12, 2014

Conversation

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Dec 12, 2014

modified:   buildbot/master/master.cfg
	modified:   buildbot/master/master.cfg
@larsbergstrom

This comment has been minimized.

Copy link

larsbergstrom commented on 3d1c522 Dec 11, 2014

This looks like a great start!

Do you know if there's a way to disable setting the GitHub status for builds run on the try branch? The scenario I'm worried about is:

  1. A PR gets a 'try' build (which passes)
  2. Conflicting semantic (but not syntactic) commits land in master
  3. bors gets an r+
  4. bors immediately notices the build passed and merges it, neglecting to re-push to auto, re-merge against master, and re-build/test

This comment has been minimized.

Copy link
Owner Author

bheesham replied Dec 15, 2014

There isn't. I've been looking into it for a few days now and there doesn't seem to be a way to have Buildbot disable status updates for a build. I asked on the IRC channel of Buildbot to make sure.

My solution to this was to create a wrapper around the StatusReceiverMultiService class, which is what GitHubStatus, IRC, etc and a few other status reporting classes all extend.

I think if I can filter it out before it gets to the status service, then we'll be in business. Ideally the only changes in the source would be to wrap GitHubStatus like this:

s['status'].append(FilteredStatus(
  GitHubStatus(
    token="",
    repoOwner="",
    repoName=""
  ),
  change_filter = change_filter_fn
))

So far it's... a work in progress. I figured I do it that way just in case an IRC or a different kind of status reporter is added.

The only downside to this is that you'll have to use a custom build for Buildbot, which isn't too bad.

This comment has been minimized.

Copy link
Owner Author

bheesham replied Dec 15, 2014

Hm, new discoveries show that it would probably be easier to do this another way. The problem I'm running into is that FilteredStatus isn't getting all the calls. Turns out I'm using the code super wrong. It's being initiated and started, but nothing else after that. 👎 Ah well. I'll keep at it and I'll let you know if I come across anything worthwhile.

@larsbergstrom
Copy link
Contributor Author

larsbergstrom commented Dec 12, 2014

Merging this to land @bheesham's changes and build on them.

larsbergstrom added a commit that referenced this pull request Dec 12, 2014
Deprioritize builds that are in the "try" branch.
@larsbergstrom larsbergstrom merged commit 3b44132 into servo:master Dec 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.