Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1753543003:
docs: try to give move precise commands in Xcode-Ninja Hybrid section (Closed)
Descriptiondocs: try to give move precise commands in Xcode-Ninja Hybrid section
In the name of making it easier for new-comers and people not very
familiar with UNIX command line, this patch tries to direct them more
explicitly, giving the exact commands they need to run in their preferred
terminal emulators.
Hopefully this should avoid more users having issues like the one posted here ->
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/LbPKQARErmM/discussion
BUG=None
TEST=visually inspected the result in http://dillinger.io/.
R=sdefresne@chromium.org
Committed: https://crrev.com/59fb57ac48e7ed753d5c479c680a9a3ce0edff46
Cr-Commit-Position: refs/heads/master@{#378772}
Patch Set 1 #
Total comments: 12
Patch Set 2 : lots of improvements per sdefresne #
Total comments: 8
Patch Set 3 : add missing blank lines between paragraphs and code blocks #Messages
Total messages: 16 (6 generated)
WDYT? I must admit that I liked the result (it seems cleaner and easier to follow, even for a long-time contributor), but my opinion might not count. ;)
Friendly ping? In case you have missed this?
Sorry, missed this CL. LGTM with some recommendation for further improvements. https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions.md File docs/mac_build_instructions.md (right): https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:147: ``` nit: for proper syntax highlighting, use ```shell https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:148: $ export GYP_GENERATORS="ninja,xcode-ninja" nit: the other pages do not prefix shell commands by $ and I think it is better (as you can just copy-n-paste them as is) https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:155: ``` ditto about ```shell and $ https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:157: ``` I would add the following: To make this change permanent, you can edit `chromium.gyp_env` (or create it if it does not exists) and define GYP\_GENERATOR\_FLAGS. In general, to use hybrid mode your `chromium.gyp_env` could contain the following: ```json { "GYP_GENERATORS" : "ninja,xcode-ninja", "GYP_GENERATOR_FLAGS": "xcode_project_version=3.2 " + "xcode_ninja_main_gyp=src/build/ninja/all.ninja.gyp", } ``` https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:160: ``` ditto about ```shell and $ https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:166: $ open build/ninja/all.ninja.xcworkspace ditto about ```shell and $
Thanks for teaching me the proper way of using the Markdown language! :) https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions.md File docs/mac_build_instructions.md (right): https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:147: ``` On 2016/03/02 14:12:18, sdefresne wrote: > nit: for proper syntax highlighting, use > > ```shell Done. https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:148: $ export GYP_GENERATORS="ninja,xcode-ninja" On 2016/03/02 14:12:18, sdefresne wrote: > nit: the other pages do not prefix shell commands by $ and I think it is better > (as you can just copy-n-paste them as is) Done. https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:155: ``` On 2016/03/02 14:12:18, sdefresne wrote: > ditto about ```shell and $ Done. https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:157: ``` On 2016/03/02 14:12:18, sdefresne wrote: > I would add the following: > > To make this change permanent, you can edit `chromium.gyp_env` (or create it if > it does not exists) and define GYP\_GENERATOR\_FLAGS. In general, to use hybrid > mode your `chromium.gyp_env` could contain the following: > > ```json > { > "GYP_GENERATORS" : "ninja,xcode-ninja", > "GYP_GENERATOR_FLAGS": > "xcode_project_version=3.2 " + > "xcode_ninja_main_gyp=src/build/ninja/all.ninja.gyp", > } > ``` Done. https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:160: ``` On 2016/03/02 14:12:18, sdefresne wrote: > ditto about ```shell and $ Done. https://codereview.chromium.org/1753543003/diff/1/docs/mac_build_instructions... docs/mac_build_instructions.md:166: $ open build/ninja/all.ninja.xcworkspace On 2016/03/02 14:12:18, sdefresne wrote: > ditto about ```shell and $ Done.
The CQ bit was checked by tfarina@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sdefresne@chromium.org Link to the patchset: https://codereview.chromium.org/1753543003/#ps20001 (title: "lots of improvements per sdefresne")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1753543003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1753543003/20001
https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... File docs/mac_build_instructions.md (right): https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... docs/mac_build_instructions.md:146: To use Xcode-Ninja Hybrid, set GYP\_GENERATORS like the following: I think you need a blank line between paragraphs and code blocs (not sure, but other file do have a blank line, see for example https://chromium.googlesource.com/chromium/src.git/+show/refs/ios/sdefresne/d...). https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... docs/mac_build_instructions.md:157: ```json Same here. https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... docs/mac_build_instructions.md:167: ```shell Here. https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... docs/mac_build_instructions.md:172: ```shell And here.
The CQ bit was unchecked by tfarina@chromium.org
https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... File docs/mac_build_instructions.md (right): https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... docs/mac_build_instructions.md:146: To use Xcode-Ninja Hybrid, set GYP\_GENERATORS like the following: On 2016/03/02 17:29:54, sdefresne wrote: > I think you need a blank line between paragraphs and code blocs (not sure, but > other file do have a blank line, see for example > https://chromium.googlesource.com/chromium/src.git/+show/refs/ios/sdefresne/d...). Acknowledged. https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... docs/mac_build_instructions.md:157: ```json On 2016/03/02 17:29:54, sdefresne wrote: > Same here. Done. https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... docs/mac_build_instructions.md:167: ```shell On 2016/03/02 17:29:54, sdefresne wrote: > Here. Done. https://codereview.chromium.org/1753543003/diff/20001/docs/mac_build_instruct... docs/mac_build_instructions.md:172: ```shell On 2016/03/02 17:29:54, sdefresne wrote: > And here. Done.
The CQ bit was checked by tfarina@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sdefresne@chromium.org Link to the patchset: https://codereview.chromium.org/1753543003/#ps40001 (title: "add missing blank lines between paragraphs and code blocks")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1753543003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1753543003/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== docs: try to give move precise commands in Xcode-Ninja Hybrid section In the name of making it easier for new-comers and people not very familiar with UNIX command line, this patch tries to direct them more explicitly, giving the exact commands they need to run in their preferred terminal emulators. Hopefully this should avoid more users having issues like the one posted here -> https://groups.google.com/a/chromium.org/d/topic/chromium-dev/LbPKQARErmM/dis... BUG=None TEST=visually inspected the result in http://dillinger.io/. R=sdefresne@chromium.org ========== to ========== docs: try to give move precise commands in Xcode-Ninja Hybrid section In the name of making it easier for new-comers and people not very familiar with UNIX command line, this patch tries to direct them more explicitly, giving the exact commands they need to run in their preferred terminal emulators. Hopefully this should avoid more users having issues like the one posted here -> https://groups.google.com/a/chromium.org/d/topic/chromium-dev/LbPKQARErmM/dis... BUG=None TEST=visually inspected the result in http://dillinger.io/. R=sdefresne@chromium.org Committed: https://crrev.com/59fb57ac48e7ed753d5c479c680a9a3ce0edff46 Cr-Commit-Position: refs/heads/master@{#378772} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/59fb57ac48e7ed753d5c479c680a9a3ce0edff46 Cr-Commit-Position: refs/heads/master@{#378772} |
