Skip to content

/ servo

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

Added support for using --features argument with --android #8176

Merged
merged 1 commit into from Oct 30, 2015
Merged

Added support for using --features argument with --android #8176

merged 1 commit into from Oct 30, 2015

Conversation

@nxnfufunezn
Copy link
Contributor

nxnfufunezn commented Oct 24, 2015

Fixes #8173
r? @mbrubeck

Review on Reviewable
@nxnfufunezn
Copy link
Contributor Author

nxnfufunezn commented Oct 25, 2015

r? @jdm
@jdm jdm assigned mbrubeck and unassigned jdm Oct 25, 2015
@jdm
Copy link
Member

jdm commented Oct 25, 2015

Matt's a fine assignee; just give him time.
@connorimes
Copy link
Contributor

connorimes commented Oct 25, 2015

I think you should add a help parameter to the CommandArgument like the others.
@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 26, 2015

Looks good! Left one additional comment below.

-S-awaiting-review +S-needs-code-changes

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

python/servo/build_commands.py, line 159 [r1] (raw file):
If features is None, we should set it to [] by default so that code below can append to it. You can do this like features = features or [].

Comments from the review on Reviewable.io
@nxnfufunezn nxnfufunezn force-pushed the nxnfufunezn:features-argument branch from 759940c to 7ca2a7c Oct 30, 2015
@nxnfufunezn nxnfufunezn force-pushed the nxnfufunezn:features-argument branch from 7ca2a7c to 4ad6eba Oct 30, 2015
@eefriedman
Copy link
Contributor

eefriedman commented Oct 30, 2015

(FYI, you just deleted the branch; you should be able to reopen the pull request once you fix that.)
@nxnfufunezn nxnfufunezn reopened this Oct 30, 2015
@nxnfufunezn
Copy link
Contributor Author

nxnfufunezn commented Oct 30, 2015

ping @mbrubeck
@nxnfufunezn nxnfufunezn force-pushed the nxnfufunezn:features-argument branch from f738368 to f1f1465 Oct 30, 2015
@nxnfufunezn
Copy link
Contributor Author

nxnfufunezn commented Oct 30, 2015

oops! changed that !
@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 30, 2015

Looks good, thanks! Please squash these commits together and force-push, and then they can land with r=mbrubeck.
@nxnfufunezn
Added support for using --features argument with --android
Loading status checks…
03bf082
@nxnfufunezn nxnfufunezn force-pushed the nxnfufunezn:features-argument branch from f1f1465 to 03bf082 Oct 30, 2015
@nxnfufunezn
Copy link
Contributor Author

nxnfufunezn commented Oct 30, 2015

@bors-servo r=mbrubeck
@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 30, 2015

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

bors-servo commented Oct 30, 2015

📌 Commit 03bf082 has been approved by mbrubeck
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2015

⌛ Testing commit 03bf082 with merge d8cb276...
bors-servo added a commit that referenced this pull request Oct 30, 2015
@bors-servo
Auto merge of #8176 - nxnfufunezn:features-argument, r=mbrubeck
Loading status checks…
d8cb276 
Added support for using --features argument with --android

Fixes #8173 
r? @mbrubeck

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8176)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2015

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit 03bf082 into servo:master Oct 30, 2015
2 of 3 checks passed
2 of 3 checks passed
code-review/reviewable Review in progress: 0 of 1 files reviewed, all discussions resolved
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Oct 30, 2015
Merged
@nxnfufunezn nxnfufunezn deleted the nxnfufunezn:features-argument branch Oct 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mbrubeck mbrubeck

Labels
A-mach L-python S-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
@nxnfufunezn @jdm @connorimes @mbrubeck @eefriedman @bors-servo @nox @highfive
You can’t perform that action at this time.