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

Read params from a file on Android #8254

Merged
merged 1 commit into from Oct 31, 2015
Merged

Read params from a file on Android #8254

merged 1 commit into from Oct 31, 2015

Conversation

@connorimes
Copy link
Contributor

connorimes commented Oct 29, 2015

Review on Reviewable
@connorimes
Copy link
Contributor Author

connorimes commented Oct 29, 2015

I just called the file android_params and loaded it with the default options, but let me know if you'd rather have something else.
@connorimes
Copy link
Contributor Author

connorimes commented Oct 29, 2015

r? @larsbergstrom
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Oct 29, 2015

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.

components/servo/main.rs, line 154 [r1] (raw file):
Maybe just make this debug log output instead of an unconditional println?

resources/android_params, line 1 [r1] (raw file):
Perhaps start with a quick bit of documentation. e.g., "one argument per line. First argument should be the servo program name"?

Comments from the review on Reviewable.io
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Oct 29, 2015

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

Comments from the review on Reviewable.io
@connorimes
Copy link
Contributor Author

connorimes commented Oct 29, 2015

Review status: all files reviewed at latest revision, 1 unresolved discussion.

components/servo/main.rs, line 154 [r1] (raw file):
The log macros were not available here and trying to add them resulted in build failures. I was originally going to make this a warn!. Do you know how I can properly add the logging macros instead?

Comments from the review on Reviewable.io
@connorimes connorimes force-pushed the connorimes:android-params-file branch 2 times, most recently from a73c2ac to 9af406a Oct 29, 2015
@connorimes
Read params from a file on Android
Loading status checks…
5a24597
@connorimes connorimes force-pushed the connorimes:android-params-file branch from 9af406a to 5a24597 Oct 30, 2015
@connorimes
Copy link
Contributor Author

connorimes commented Oct 30, 2015

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.

components/servo/main.rs, line 154 [r1] (raw file):
After rebasing, this particular problem has gone away. It's now using a debug! message.

Comments from the review on Reviewable.io
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Oct 30, 2015

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Oct 30, 2015

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

bors-servo commented Oct 30, 2015

📌 Commit 5a24597 has been approved by larsbergstrom
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2015

⌛ Testing commit 5a24597 with merge 7b1bcaa...
bors-servo added a commit that referenced this pull request Oct 30, 2015
@bors-servo
Auto merge of #8254 - connorimes:android-params-file, r=larsbergstrom
7b1bcaa 
Read params from a file on Android



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

bors-servo commented Oct 30, 2015

💔 Test failed - mac-dev-ref-unit
@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 30, 2015

@bors-servo retry

#7785
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2015

⌛ Testing commit 5a24597 with merge 1b7f9c9...
bors-servo added a commit that referenced this pull request Oct 30, 2015
@bors-servo
Auto merge of #8254 - connorimes:android-params-file, r=larsbergstrom
1b7f9c9 
Read params from a file on Android



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

bors-servo commented Oct 30, 2015

💔 Test failed - mac-dev-ref-unit
@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 30, 2015

@bors-servo retry

#7785
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2015

⌛ Testing commit 5a24597 with merge 42aab9e...
bors-servo added a commit that referenced this pull request Oct 30, 2015
@bors-servo
Auto merge of #8254 - connorimes:android-params-file, r=larsbergstrom
42aab9e 
Read params from a file on Android



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

bors-servo commented Oct 30, 2015

💔 Test failed - mac-dev-ref-unit
@eefriedman
Copy link
Contributor

eefriedman commented Oct 30, 2015

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2015

⚡ Previous build results for android, gonk, linux-dev, mac-rel-css are reusable. Rebuilding only linux-rel, mac-dev-ref-unit, mac-rel-wpt...
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2015

💔 Test failed - mac-dev-ref-unit
@eefriedman
Copy link
Contributor

eefriedman commented Oct 30, 2015

@bors-servo retry #7785
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2015

⌛ Testing commit 5a24597 with merge 2d0d9ca...
bors-servo added a commit that referenced this pull request Oct 30, 2015
@bors-servo
Auto merge of #8254 - connorimes:android-params-file, r=larsbergstrom
Loading status checks…
2d0d9ca 
Read params from a file on Android



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

bors-servo commented Oct 31, 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 5a24597 into servo:master Oct 31, 2015
3 checks passed
3 checks passed
code-review/reviewable Review complete: all files reviewed, all discussions resolved
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@larsbergstrom larsbergstrom

Labels
S-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@connorimes @larsbergstrom @bors-servo @mbrubeck @eefriedman @highfive
You can’t perform that action at this time.