Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2079953002:
Added HACKING.md to observatory (Closed)
|
Created:
4 years, 6 months ago by cbernaschina Modified:
4 years, 5 months ago CC:
reviews_dartlang.org, turnidge, rmacnak, Cutch, vm-dev_dartlang.org Base URL:
https://github.com/dart-lang/sdk.git@master Target Ref:
refs/heads/master Visibility:
Public. |
DescriptionAdded HACKING.md to observatory
R=johnmccutchan@google.com
Committed: https://github.com/dart-lang/sdk/commit/a601b65261d9cf309109d77bfebedf0d26f1c450
Patch Set 1 #
Total comments: 2
Patch Set 2 : Reduced column width to 80 where possible #
Total comments: 12
Patch Set 3 : Updated HACKING.md #
Total comments: 8
Patch Set 4 : Fixed last typos #Messages
Total messages: 10 (2 generated)
cbernaschina@google.com changed reviewers: + johnmccutchan@google.com, rmacnak@google.com, turnidge@google.com
https://codereview.chromium.org/2079953002/diff/1/runtime/observatory/HACKING.md File runtime/observatory/HACKING.md (right): https://codereview.chromium.org/2079953002/diff/1/runtime/observatory/HACKING... runtime/observatory/HACKING.md:3: These instructions will guide you through the Observatory development and testing workflow. (here and elsewhere) lines must be <= 80 columns long
https://codereview.chromium.org/2079953002/diff/1/runtime/observatory/HACKING.md File runtime/observatory/HACKING.md (right): https://codereview.chromium.org/2079953002/diff/1/runtime/observatory/HACKING... runtime/observatory/HACKING.md:3: These instructions will guide you through the Observatory development and testing workflow. On 2016/06/20 14:06:12, Cutch wrote: > (here and elsewhere) lines must be <= 80 columns long Done.
More comments https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... File runtime/observatory/HACKING.md (right): https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:9: Before starting hacking observatory follow [the instructions][1] to have a how about [instructions](http://....) for this and all the links? https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:17: $ ./tools/test.py -mrelease service We recommend people use the debug mode for running tests. Also, they will need to have built the debug mode first. https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:21: If you want to avoid to trigger a new compilation to JavaScript for each edit If you want to avoid triggering a ... https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:25: You can obtain Dartium in two different ways: Move the dartium download up to the top in the setup & build section. https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:30: In order to avoid a complete build of the sdk for each change you do to How about: "Observatory is built as part of building the sdk, but when working on Observatory we recommend that you use pub serve so you can avoid the overhead of building the sdk for each change" https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:47: [get started guide][5]) and connect your Observatory instance to that VM. Let's provide a sample script here: ```dart import 'dart:async'; main() { new Timer.periodic(const Duration(seconds: 1..... } ``` And then give the example command line: $ dart --observe scrpit.dart
https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... File runtime/observatory/HACKING.md (right): https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:9: Before starting hacking observatory follow [the instructions][1] to have a On 2016/06/20 14:47:01, Cutch wrote: > how about [instructions](http://....) for this and all the links? The [...][...] version allows to reuse links and to force a tooltip. Anyway it is just a personal preference. https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:17: $ ./tools/test.py -mrelease service On 2016/06/20 14:47:01, Cutch wrote: > We recommend people use the debug mode for running tests. > > Also, they will need to have built the debug mode first. Done. https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:21: If you want to avoid to trigger a new compilation to JavaScript for each edit On 2016/06/20 14:47:01, Cutch wrote: > If you want to avoid triggering a ... Done. https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:25: You can obtain Dartium in two different ways: On 2016/06/20 14:47:01, Cutch wrote: > Move the dartium download up to the top in the setup & build section. Done. https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:30: In order to avoid a complete build of the sdk for each change you do to On 2016/06/20 14:47:01, Cutch wrote: > How about: > > "Observatory is built as part of building the sdk, but when working on > Observatory we recommend that you use pub serve so you can avoid the overhead of > building the sdk for each change" Done. https://codereview.chromium.org/2079953002/diff/20001/runtime/observatory/HAC... runtime/observatory/HACKING.md:47: [get started guide][5]) and connect your Observatory instance to that VM. On 2016/06/20 14:47:01, Cutch wrote: > Let's provide a sample script here: > > > ```dart > import 'dart:async'; > > main() { > new Timer.periodic(const Duration(seconds: 1..... > } > ``` > > And then give the example command line: > > $ dart --observe scrpit.dart Done.
LGTM after comments are addressed https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... File runtime/observatory/HACKING.md (right): https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... runtime/observatory/HACKING.md:9: Before starting hacking observatory follow the [instructions][build_sdk] to have a Before you start to hack on Observatory, follow the .... https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... runtime/observatory/HACKING.md:9: Before starting hacking observatory follow the [instructions][build_sdk] to have a (here and elsewhere) reflow to 80 character limit. I suggest fixing your text editor to display a vertical line there. https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... runtime/observatory/HACKING.md:28: $ ./tools/build.py --mode debug --arch x64 create_sdk drop the --arch flag here or add it to test.py (the -m and -a flags passed to both of them must be consistent). https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... runtime/observatory/HACKING.md:132: code() async{ missing space after async
Should I wait a LGTM from turnidge & rmacnak? https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... File runtime/observatory/HACKING.md (right): https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... runtime/observatory/HACKING.md:9: Before starting hacking observatory follow the [instructions][build_sdk] to have a On 2016/06/21 20:02:07, Cutch wrote: > Before you start to hack on Observatory, follow the .... Done. https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... runtime/observatory/HACKING.md:9: Before starting hacking observatory follow the [instructions][build_sdk] to have a On 2016/06/21 20:02:07, Cutch wrote: > (here and elsewhere) reflow to 80 character limit. I suggest fixing your text > editor to display a vertical line there. Done. https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... runtime/observatory/HACKING.md:28: $ ./tools/build.py --mode debug --arch x64 create_sdk On 2016/06/21 20:02:07, Cutch wrote: > drop the --arch flag here or add it to test.py (the -m and -a flags passed to > both of them must be consistent). Done. https://codereview.chromium.org/2079953002/diff/40001/runtime/observatory/HAC... runtime/observatory/HACKING.md:132: code() async{ On 2016/06/21 20:02:07, Cutch wrote: > missing space after async Done.
Description was changed from ========== Added HACKING.md to observatory ========== to ========== Added HACKING.md to observatory R=johnmccutchan@google.com Committed: https://github.com/dart-lang/sdk/commit/a601b65261d9cf309109d77bfebedf0d26f1c450 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) manually as a601b65261d9cf309109d77bfebedf0d26f1c450 (presubmit successful). |
