Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2910523002:
update changelog to mention pub support for dartdevc (Closed)
Descriptionupdate changelog to mention pub support for dartdevc
BUG=
R=nweiz@google.com
Committed: https://github.com/dart-lang/sdk/commit/1d9a2e8a950b96bd3296795746f07e08c666b7b6
Patch Set 1 #
Total comments: 8
Patch Set 2 : additional updates #
Total comments: 8
Patch Set 3 : code review updates #Messages
Total messages: 11 (2 generated)
jakemac@google.com changed reviewers: + nweiz@google.com
Note that I won't merge this until https://github.com/dart-lang/pub/pull/1624 is merged since we changed the command line arg.
Looking at the pub log since 1.23.0, it looks like there are a few more changes that need to be documented: * http://github.com/dart-lang/pub/commit/bd769e90 (?) * http://github.com/dart-lang/pub/commit/a6fc90b2 * http://github.com/dart-lang/pub/commit/153a67e3 https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md File CHANGELOG.md (right): https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode80 CHANGELOG.md:80: opting in: Nit: it would read easier in plain text to add some whitespace between the different bullet points here. https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode80 CHANGELOG.md:80: opting in: Mention which pub commands are affected. https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode83 CHANGELOG.md:83: out without changing the default. Provide some more detail here about what exact behavior this affects. Remember, this is the only documentation people have for this for the crucial time period where they can try it out and suggest breaking changes. https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode94 CHANGELOG.md:94: ``` It's probably best to wait to mention this until it's landed in the SDK.
On 2017/05/25 19:54:13, nweiz wrote: > Looking at the pub log since 1.23.0, it looks like there are a few more changes > that need to be documented: > > * http://github.com/dart-lang/pub/commit/bd769e90 (?) > * http://github.com/dart-lang/pub/commit/a6fc90b2 > * http://github.com/dart-lang/pub/commit/153a67e3 > > https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md > File CHANGELOG.md (right): > > https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode80 > CHANGELOG.md:80: opting in: > Nit: it would read easier in plain text to add some whitespace between the > different bullet points here. > > https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode80 > CHANGELOG.md:80: opting in: > Mention which pub commands are affected. > > https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode83 > CHANGELOG.md:83: out without changing the default. > Provide some more detail here about what exact behavior this affects. Remember, > this is the only documentation people have for this for the crucial time period > where they can try it out and suggest breaking changes. > > https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode94 > CHANGELOG.md:94: ``` > It's probably best to wait to mention this until it's landed in the SDK. Added docs for http://github.com/dart-lang/pub/commit/a6fc90b2 and http://github.com/dart-lang/pub/commit/153a67e3, the other one doesn't have any user facing changes.
https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md File CHANGELOG.md (right): https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode80 CHANGELOG.md:80: opting in: On 2017/05/25 19:54:13, nweiz wrote: > Mention which pub commands are affected. Done. https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode80 CHANGELOG.md:80: opting in: On 2017/05/25 19:54:13, nweiz wrote: > Nit: it would read easier in plain text to add some whitespace between the > different bullet points here. Done. https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode83 CHANGELOG.md:83: out without changing the default. On 2017/05/25 19:54:13, nweiz wrote: > Provide some more detail here about what exact behavior this affects. Remember, > this is the only documentation people have for this for the crucial time period > where they can try it out and suggest breaking changes. Done. https://codereview.chromium.org/2910523002/diff/1/CHANGELOG.md#newcode94 CHANGELOG.md:94: ``` On 2017/05/25 19:54:13, nweiz wrote: > It's probably best to wait to mention this until it's landed in the SDK. I was just going to wait to merge this until it is in the sdk, updating it more than once seems redundant (and also the flag names here would need to change).
https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md File CHANGELOG.md (right): https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md#newcode86 CHANGELOG.md:86: chrome (or other supported browsers), and see your edits almost Nit: "Chrome" (also below) https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md#newcode98 CHANGELOG.md:98: `dart2js` or `none` as options. This is the easiest way to try things What does "none" do? https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md#newcode102 CHANGELOG.md:102: single key called `compiler`. This is a map of string to string, where "of string to string..." -> "from mode names to the compilers to use" https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md#newcode115 CHANGELOG.md:115: your test directory `pub serve test --web-compiler=dartdevc`, and then run -"pub serve on your test directory"
https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md File CHANGELOG.md (right): https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md#newcode86 CHANGELOG.md:86: chrome (or other supported browsers), and see your edits almost On 2017/05/26 22:30:09, nweiz wrote: > Nit: "Chrome" (also below) Done. https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md#newcode98 CHANGELOG.md:98: `dart2js` or `none` as options. This is the easiest way to try things On 2017/05/26 22:30:10, nweiz wrote: > What does "none" do? It is equivalent to --no-dart2js (no js compilation at all). I added a deprecation notice for --no-dart2js as a separate bullet point. https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md#newcode102 CHANGELOG.md:102: single key called `compiler`. This is a map of string to string, where On 2017/05/26 22:30:09, nweiz wrote: > "of string to string..." -> "from mode names to the compilers to use" Done. https://codereview.chromium.org/2910523002/diff/20001/CHANGELOG.md#newcode115 CHANGELOG.md:115: your test directory `pub serve test --web-compiler=dartdevc`, and then run On 2017/05/26 22:30:09, nweiz wrote: > -"pub serve on your test directory" Done.
lgtm
Description was changed from ========== update changelog to mention pub support for dartdevc BUG= ========== to ========== update changelog to mention pub support for dartdevc BUG= R=nweiz@google.com Committed: https://github.com/dart-lang/sdk/commit/1d9a2e8a950b96bd3296795746f07e08c666b7b6 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as 1d9a2e8a950b96bd3296795746f07e08c666b7b6 (presubmit successful). |
