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

Move buildbot step to yaml #10849

Merged
merged 1 commit into from Apr 28, 2016
Merged

Move buildbot step to yaml #10849

merged 1 commit into from Apr 28, 2016

Conversation

@shinglyu
Copy link
Member

shinglyu commented Apr 26, 2016

This is a step of servo/saltfs#316

After this patch lands, we'll PR the saltfs code to read from this yaml file, and dynamically generate test steps at runtime.

cc @aneeshusa @edunham


This change is Reviewable

@wafflespeanut
Copy link
Member

wafflespeanut commented Apr 26, 2016

@nox
Copy link
Member

nox commented Apr 26, 2016

I'm confused. Since the YAML change, how does Buildbot know the difference between a compile step and a test step?

@aneeshusa
Copy link
Member

aneeshusa commented Apr 26, 2016

@nox Heuristics, based on the command given to mach. (e.g. build-cef vs test-wpt).
One of the goals of switching to YAML was to decrease the complexity of changing build steps to make it easier to contribute to infra, so I used heuristics instead of hardcoding many things. This allows for a simple list of strings, instead of requiring each line to be a dictionary with type: Compile, etc. keys.

Also, Test and Compile steps have essentially the same behavior, so in practice the difference is moot. The most visible difference right now is what they're labeled on the Buildbot display, which should improve with servo/saltfs#337.

@aneeshusa
Copy link
Member

aneeshusa commented Apr 27, 2016

Can you update this to include the changes from servo/saltfs#341?

@nox
Copy link
Member

nox commented Apr 27, 2016

@aneeshusa AFAIK buildbot stops on a compile step failing to succeed, while it doesn't on a test step.

@aneeshusa
Copy link
Member

aneeshusa commented Apr 27, 2016

@nox You're right, I completely overlooked that. Seems like I learn something new about Buildbot every day!

@shinglyu shinglyu force-pushed the shinglyu:yaml branch from ef7a5e9 to c01aa9c Apr 28, 2016
@shinglyu
Copy link
Member Author

shinglyu commented Apr 28, 2016

@aneeshusa I just force pushed a version with servo/saltfs#341

@aneeshusa
Copy link
Member

aneeshusa commented Apr 28, 2016

@bors-servo r+

@shinglyu Thanks! Let me know if you have any questions while making the other PR.

@bors-servo
Copy link
Contributor

bors-servo commented Apr 28, 2016

📌 Commit c01aa9c has been approved by aneeshusa

@bors-servo
Copy link
Contributor

bors-servo commented Apr 28, 2016

💡 This pull request was already approved, no need to approve it again.

@bors-servo
Copy link
Contributor

bors-servo commented Apr 28, 2016

📌 Commit c01aa9c has been approved by aneeshusa

@bors-servo
Copy link
Contributor

bors-servo commented Apr 28, 2016

Testing commit c01aa9c with merge fbc5754...

bors-servo added a commit that referenced this pull request Apr 28, 2016
Move buildbot step to yaml

This is a step of servo/saltfs#316

After this patch lands, we'll PR the saltfs code to read from this yaml file, and dynamically generate test steps at runtime.

cc @aneeshusa @edunham

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10849)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 28, 2016

@bors-servo bors-servo merged commit c01aa9c into servo:master Apr 28, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@edunham edunham mentioned this pull request Apr 28, 2016
4 of 4 tasks complete
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

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