Update HACKING_QUICKSTART.md #12266

tshepang · 2016-07-05T16:17:51Z

Some grammar fixes

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #__ (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because they are documentation changes

This change is

highfive · 2016-07-05T16:17:57Z

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @nox (or someone else) soon.

nox · 2016-07-05T16:21:29Z

-S-awaiting-review +S-awaiting-answer

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

docs/HACKING_QUICKSTART.md, line 12 [r1] (raw file):

## Building Servo

Building Servo is quite easy. Install the pre-requisites described in the [README](../README.md) file, then type:

"Prerequisites" exists and seems to be more widely-used, no?

docs/HACKING_QUICKSTART.md, line 48 [r1] (raw file):

The -- separates mach options from servo options. This is not required, but we recommend it. mach and servo have some options with the same name (--help, --debug), so the -- makes it clear where options apply.

Why?

docs/HACKING_QUICKSTART.md, line 244 [r1] (raw file):

### Updating a test:

In some cases, extensive tests for the feature you're working on already exist under tests/wpt:

Why remove "may"?

Comments from Reviewable

tshepang · 2016-07-05T16:40:35Z

Review status: all files reviewed at latest revision, 3 unresolved discussions.

docs/HACKING_QUICKSTART.md, line 12 [r1] (raw file):

Previously, nox (Anthony Ramine) wrote…

"Prerequisites" exists and seems to be more widely-used, no?

agreed... I thought it was the other way around

docs/HACKING_QUICKSTART.md, line 48 [r1] (raw file):

Previously, nox (Anthony Ramine) wrote…

Why?

One should not continue the sentence like that, so adding _so_ makes it flow better.

docs/HACKING_QUICKSTART.md, line 244 [r1] (raw file):

Previously, nox (Anthony Ramine) wrote…

Why remove "may"?

You either want to remove _may_ or _In some cases__. That is, in this case, *In some cases_ implies _sometimes_, same as _may_.

Comments from Reviewable

nox · 2016-07-05T16:42:11Z

Thanks for your contribution!

-S-awaiting-answer +S-needs-code-changes

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

docs/HACKING_QUICKSTART.md, line 12 [r1] (raw file):

Previously, tshepang (Tshepang Lekhonkhobe) wrote…

agreed... I thought it was the other way around

r=me once this is reverted. :)

Comments from Reviewable

tshepang · 2016-07-06T16:40:52Z

r=@nox

KiChjang · 2016-07-06T16:41:56Z

Unfortunately, bors-servo only listens to reviewers.

@bors-servo r=nox

bors-servo · 2016-07-06T16:41:57Z

📌 Commit 7573487 has been approved by nox

KiChjang · 2016-07-06T17:18:09Z

@bors-servo r-

@tshepang Sorry, can I get you to squash your two commits into one before I accept this PR?

tshepang · 2016-07-06T21:48:11Z

Does the UI not allow you to do it? Let me do it from my CLI.


        doc: small fixes to HACKING_QUICKSTART.md

jdm · 2016-07-07T02:44:35Z

@bors-servo: r=nox
The github UI does, but our merge bot does not.

bors-servo · 2016-07-07T02:44:36Z

📌 Commit 9283770 has been approved by nox

bors-servo · 2016-07-07T02:47:38Z

⌛ Testing commit 9283770 with merge 1d8b04f...


        Auto merge of #12266 - tshepang:patch-1, r=nox

Update HACKING_QUICKSTART.md Some grammar fixes ---  - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [X] These changes do not require tests because they are documentation changes   --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12266)

bors-servo · 2016-07-07T03:49:53Z

💔 Test failed - linux-rel

wafflespeanut · 2016-07-07T07:25:08Z

@bors-servo retry #11703

bors-servo · 2016-07-07T07:26:35Z

⌛ Testing commit 9283770 with merge 220b0ef...


        Auto merge of #12266 - tshepang:patch-1, r=nox

Update HACKING_QUICKSTART.md Some grammar fixes ---  - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [X] These changes do not require tests because they are documentation changes   --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12266)

bors-servo · 2016-07-07T08:16:40Z

💔 Test failed - mac-rel-wpt

wafflespeanut · 2016-07-07T08:19:44Z

This is happening again!

Traceback (most recent call last):
  File "/Users/servo/buildbot/slave/mac-rel-wpt/build/tests/wpt/harness/wptrunner/executors/base.py", line 149, in run_test
    result = self.do_test(test)
  File "/Users/servo/buildbot/slave/mac-rel-wpt/build/tests/wpt/harness/wptrunner/executors/executorservo.py", line 133, in do_test
    self.proc.kill()
  File "/Users/servo/buildbot/slave/mac-rel-wpt/build/python/_virtualenv/lib/python2.7/site-packages/mozprocess/processhandler.py", line 749, in kill
    self.proc.kill(sig=sig)
  File "/Users/servo/buildbot/slave/mac-rel-wpt/build/python/_virtualenv/lib/python2.7/site-packages/mozprocess/processhandler.py", line 165, in kill
    send_sig(signal.SIGTERM)
  File "/Users/servo/buildbot/slave/mac-rel-wpt/build/python/_virtualenv/lib/python2.7/site-packages/mozprocess/processhandler.py", line 152, in send_sig
    os.killpg(pid, sig)
OSError: [Errno 1] Operation not permitted

KiChjang · 2016-07-07T08:47:48Z

@bors-servo retry

#InfraOnFire

KiChjang · 2016-07-07T08:53:38Z

@bors-servo rollup

tshepang · 2016-07-07T11:11:57Z

@jdm you could squash via Github UI then ask the bot to merge?

wafflespeanut · 2016-07-07T11:15:37Z

We never use the Github UI to merge commits. We've been relying on bors's auto-merging mechanism for a long time. Also, testing helps us to keep track of the intermittents that pop up every now and then.

tshepang · 2016-07-07T13:22:19Z

@wafflespeanut understood, but that's not what I said

KiChjang · 2016-07-07T13:49:13Z

@tshepang I only see the "Squash and Merge" option. I don't think GitHub UI provides a way for us to only perform a squash.

tshepang · 2016-07-07T21:35:41Z

oh, alright

bors-servo · 2016-07-08T07:51:04Z

⌛ Testing commit 9283770 with merge 9b2b101...


        Auto merge of #12266 - tshepang:patch-1, r=nox

Update HACKING_QUICKSTART.md Some grammar fixes ---  - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [X] These changes do not require tests because they are documentation changes   --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12266)

bors-servo · 2016-07-08T08:45:51Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

highfive assigned nox Jul 5, 2016

highfive added the S-awaiting-review label Jul 5, 2016

nox added S-awaiting-answer and removed S-awaiting-review labels Jul 5, 2016

nox added S-needs-code-changes and removed S-awaiting-answer labels Jul 5, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Jul 6, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Jul 6, 2016

KiChjang added S-needs-squash and removed S-awaiting-merge labels Jul 6, 2016

doc: small fixes to HACKING_QUICKSTART.md

Loading status checks…

9283770

tshepang force-pushed the tshepang:patch-1 branch from 7573487 to 9283770 Jul 6, 2016

highfive added the S-awaiting-review label Jul 6, 2016

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-squash labels Jul 7, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jul 7, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Jul 7, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jul 7, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Jul 8, 2016

bors-servo merged commit 9283770 into servo:master Jul 8, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jul 8, 2016

tshepang deleted the tshepang:patch-1 branch Jul 8, 2016

servo / servo

Update HACKING_QUICKSTART.md #12266

Update HACKING_QUICKSTART.md #12266

tshepang commented Jul 5, 2016 •

edited by larsbergstrom

highfive commented Jul 5, 2016

nox commented Jul 5, 2016

tshepang commented Jul 5, 2016

nox commented Jul 5, 2016

tshepang commented Jul 6, 2016

KiChjang commented Jul 6, 2016

bors-servo commented Jul 6, 2016

KiChjang commented Jul 6, 2016

tshepang commented Jul 6, 2016

jdm commented Jul 7, 2016

bors-servo commented Jul 7, 2016

bors-servo commented Jul 7, 2016

bors-servo commented Jul 7, 2016

wafflespeanut commented Jul 7, 2016

bors-servo commented Jul 7, 2016

bors-servo commented Jul 7, 2016

wafflespeanut commented Jul 7, 2016

KiChjang commented Jul 7, 2016

KiChjang commented Jul 7, 2016

tshepang commented Jul 7, 2016

wafflespeanut commented Jul 7, 2016

tshepang commented Jul 7, 2016

KiChjang commented Jul 7, 2016

tshepang commented Jul 7, 2016

bors-servo commented Jul 8, 2016

bors-servo commented Jul 8, 2016

servo / servo

Join GitHub today

Update HACKING_QUICKSTART.md #12266

Update HACKING_QUICKSTART.md #12266

Conversation

tshepang commented Jul 5, 2016 • edited by larsbergstrom

highfive commented Jul 5, 2016

nox commented Jul 5, 2016

tshepang commented Jul 5, 2016

nox commented Jul 5, 2016

tshepang commented Jul 6, 2016

KiChjang commented Jul 6, 2016

bors-servo commented Jul 6, 2016

KiChjang commented Jul 6, 2016

tshepang commented Jul 6, 2016

jdm commented Jul 7, 2016

bors-servo commented Jul 7, 2016

bors-servo commented Jul 7, 2016

bors-servo commented Jul 7, 2016

wafflespeanut commented Jul 7, 2016

bors-servo commented Jul 7, 2016

bors-servo commented Jul 7, 2016

wafflespeanut commented Jul 7, 2016

KiChjang commented Jul 7, 2016

KiChjang commented Jul 7, 2016

tshepang commented Jul 7, 2016

wafflespeanut commented Jul 7, 2016

tshepang commented Jul 7, 2016

KiChjang commented Jul 7, 2016

tshepang commented Jul 7, 2016

bors-servo commented Jul 8, 2016

bors-servo commented Jul 8, 2016

tshepang commented Jul 5, 2016 •

edited by larsbergstrom