Skip to content

/ saltfs

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 trychooser support #873

Merged
merged 3 commits into from Aug 9, 2018
Merged

Add trychooser support #873

merged 3 commits into from Aug 9, 2018
+34 −2

Conversation

@Manishearth
Copy link
Member

Manishearth commented Aug 1, 2018 •
edited

Depends on servo/homu#167

homu SQLite migration notes: Either start from a fresh db, or run ALTER TABLE pull ADD try_choose text

r? @jdm

This change is Reviewable
@aneeshusa
aneeshusa reviewed Aug 4, 2018
View changes
Copy link
Member

aneeshusa left a comment

r=me after nit, but let's not merge until #873 is merged to master.
buildbot/master/files/config/master.cfg Outdated
@@ -60,6 +60,13 @@ def servo_master_filter(c):
c.who.startswith('bors-servo') and
c.branch == "master")

def servo_try_chooser_filter(f):
branch = "try-%s" % f

This comment has been minimized.

Sign in to view
@aneeshusa

aneeshusa Aug 4, 2018

Member

nit: inline this
@Manishearth Manishearth force-pushed the Manishearth:trychooser branch 4 times, most recently from 093333c to 08ccca1 Aug 5, 2018
@jdm
jdm requested changes Aug 8, 2018
View changes
homu/map.jinja Outdated
@@ -1,6 +1,6 @@
{%
set homu = {
'db': '/var/homu/main.db',
'rev': '992e774d7144e810193bcaede827339663c1301a'
'db': '/var/homu/main_test.db',

This comment has been minimized.

Sign in to view
@jdm

jdm Aug 8, 2018

Member

This should be reverted.
homu/files/cfg.toml Outdated
[repo.servo.buildbot.try_choosers]
build = ["linux-dev"]
wpt = ["linux-rel-css", "linux-rel-wpt"]
android = ["android"]

This comment has been minimized.

Sign in to view
@jdm

jdm Aug 8, 2018

Member

This should include android-x86 as well.
homu/map.jinja Outdated
'servo_try_choosers': {
build': ['linux-dev'],
'wpt': ['linux-rel-css', 'linux-rel-wpt']
'android': ['android']

This comment has been minimized.

Sign in to view
@jdm

jdm Aug 8, 2018

Member

Let's add android-x86.
homu/files/cfg.toml Outdated
@@ -240,6 +240,11 @@ try_builders = [
username = "{{ secrets["buildbot-http-user"] }}"
password = "{{ secrets["buildbot-http-pass"] }}"

{% for chooser, builders in homu.try_choosers.items() %}
[repo.servo.buildbot."{{ chooser }}"]

This comment has been minimized.

Sign in to view
@jdm

jdm Aug 8, 2018

Member

Shouldn't this be buildbot.try_choosers."{{ chooser }}"?
@Manishearth
Add trychooser support
18368b3
@Manishearth Manishearth force-pushed the Manishearth:trychooser branch from 08ccca1 to c3d859c Aug 8, 2018
@Manishearth
Copy link
Member Author

Manishearth commented Aug 8, 2018

Done.
@Manishearth Manishearth force-pushed the Manishearth:trychooser branch 2 times, most recently from 83add7c to 0833f8d Aug 8, 2018
@jdm
jdm approved these changes Aug 8, 2018
View changes
@Manishearth
Move try chooser list to jinja
Loading status checks…
cf9c717
@Manishearth Manishearth force-pushed the Manishearth:trychooser branch from 0833f8d to cf9c717 Aug 8, 2018
@Manishearth
Copy link
Member Author

Manishearth commented Aug 9, 2018

The array doesn't format quite right so I explicitly printed it out instead of relying on the stringifier.
@Manishearth
Copy link
Member Author

Manishearth commented Aug 9, 2018

It now seems to work on vagrant
@jdm
Copy link
Member

jdm commented Aug 9, 2018 

[ FAIL ] Buildbot master config lint check failed:
         ./buildbot/master/files/config/master.cfg:63:1: E302 expected 2 blank lines, found 1
         ./buildbot/master/files/config/master.cfg:96:2: E225 missing whitespace around operator
         ./buildbot/master/files/config/master.cfg:96:3: E901 SyntaxError: invalid syntax
         ./buildbot/master/files/config/master.cfg:96:62: E225 missing whitespace around operator
         ./buildbot/master/files/config/master.cfg:101:80: E501 line too long (89 > 79 characters)
         ./buildbot/master/files/config/master.cfg:103:2: E225 missing whitespace around operator
         ./buildbot/master/files/config/master.cfg:103:12: E225 missing whitespace around operator
@Manishearth Manishearth force-pushed the Manishearth:trychooser branch from cdea5e4 to b4bd8c2 Aug 9, 2018
@jdm
Copy link
Member

jdm commented Aug 9, 2018 

[ FAIL ] Buildbot master config lint check failed:
         home/servo/buildbot/master/master.cfg:1:1: E902 FileNotFoundError: [Errno 2] No such file or directory: 'home/servo/buildbot/master/master.cfg'
@jdm
jdm reviewed Aug 9, 2018
View changes
tests/sls/buildbot/master/config_lint.py Outdated
@@ -13,7 +13,7 @@ def run():
'config'
)
# Have to specify master.cfg separately because it is not a .py file
command = ['flake8', CONF_DIR, os.path.join(CONF_DIR, 'master.cfg')]
command = ['flake8', CONF_DIR, "home/servo/buildbot/master/master.cfg"]

This comment has been minimized.

Sign in to view
@jdm

jdm Aug 9, 2018

Member

Revert this change?

This comment has been minimized.

Sign in to view
@Manishearth

Manishearth Aug 9, 2018

Author Member

No, this is intentional, this lets us lint the generated file.

The other option is to locally jinja it but I tried it and the salt jinja format is weird so I'd have to convert first.
@Manishearth Manishearth force-pushed the Manishearth:trychooser branch from b4bd8c2 to 39f674b Aug 9, 2018
@Manishearth
Lint generated master.cfg
Loading status checks…
f5d1c44
@Manishearth Manishearth force-pushed the Manishearth:trychooser branch from 39f674b to f5d1c44 Aug 9, 2018
@jdm
Copy link
Member

jdm commented Aug 9, 2018

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Aug 9, 2018

📌 Commit f5d1c44 has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Aug 9, 2018

⌛ Testing commit f5d1c44 with merge f17dde9...
bors-servo added a commit that referenced this pull request Aug 9, 2018
@bors-servo
Auto merge of #873 - Manishearth:trychooser, r=jdm
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
f17dde9 
Add trychooser support

Depends on servo/homu#167

homu SQLite migration notes: Either start from a fresh db, or run `ALTER TABLE pull ADD try_choose text`

r? @jdm

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

bors-servo commented Aug 9, 2018

☀️ Test successful - status-travis
Approved by: jdm
Pushing f17dde9 to master...
@bors-servo bors-servo merged commit f5d1c44 into servo:master Aug 9, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@jdm jdm removed the S-needs-deploy label Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm

@aneeshusa aneeshusa

Assignees

@jdm jdm

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@Manishearth @jdm @bors-servo @aneeshusa @highfive
You can’t perform that action at this time.