Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(57)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 11638010: Convert /** comments to /// in pub. (Closed)

Created:
8 years ago by Bob Nystrom
Modified:
8 years ago
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Convert /** comments to /// in pub. Previously, it was a mix of both styles. Now it's consistent. :) Committed: https://code.google.com/p/dart/source/detail?r=16354

Patch Set 1 #

Patch Set 2 : Fix 80 column limit. #

Total comments: 18

Patch Set 3 : Respond to review. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+878 lines, -1270 lines) Patch
A tools/line_doc_comments.dart View 1 2 1 chunk +87 lines, -0 lines 0 comments Download
M utils/pub/command_help.dart View 1 chunk +1 line, -1 line 0 comments Download
M utils/pub/command_install.dart View 1 chunk +1 line, -1 line 0 comments Download
M utils/pub/command_update.dart View 1 chunk +1 line, -1 line 0 comments Download
M utils/pub/command_version.dart View 2 chunks +2 lines, -2 lines 0 comments Download
M utils/pub/entrypoint.dart View 1 2 11 chunks +59 lines, -93 lines 0 comments Download
M utils/pub/git.dart View 2 chunks +2 lines, -4 lines 0 comments Download
M utils/pub/git_source.dart View 1 10 chunks +48 lines, -75 lines 0 comments Download
M utils/pub/hosted_source.dart View 1 2 6 chunks +21 lines, -37 lines 0 comments Download
M utils/pub/http.dart View 1 2 2 chunks +3 lines, -7 lines 0 comments Download
M utils/pub/io.dart View 1 21 chunks +65 lines, -113 lines 0 comments Download
M utils/pub/lock_file.dart View 3 chunks +4 lines, -12 lines 0 comments Download
M utils/pub/oauth2.dart View 1 chunk +1 line, -1 line 0 comments Download
M utils/pub/package.dart View 1 3 chunks +39 lines, -81 lines 0 comments Download
M utils/pub/pub.dart View 7 chunks +13 lines, -27 lines 0 comments Download
M utils/pub/pubspec.dart View 3 chunks +8 lines, -18 lines 0 comments Download
M utils/pub/root_source.dart View 1 2 1 chunk +4 lines, -6 lines 0 comments Download
M utils/pub/source.dart View 1 2 2 chunks +94 lines, -124 lines 0 comments Download
M utils/pub/source_registry.dart View 1 3 chunks +10 lines, -24 lines 0 comments Download
M utils/pub/system_cache.dart View 1 2 chunks +17 lines, -30 lines 0 comments Download
M utils/pub/utils.dart View 1 8 chunks +15 lines, -32 lines 0 comments Download
M utils/pub/version.dart View 1 11 chunks +49 lines, -73 lines 0 comments Download
M utils/pub/version_solver.dart View 1 21 chunks +118 lines, -194 lines 0 comments Download
M utils/pub/yaml/composer.dart View 1 2 9 chunks +23 lines, -33 lines 0 comments Download
M utils/pub/yaml/constructor.dart View 1 2 3 chunks +12 lines, -16 lines 0 comments Download
M utils/pub/yaml/deep_equals.dart View 4 chunks +6 lines, -10 lines 0 comments Download
M utils/pub/yaml/model.dart View 1 2 8 chunks +31 lines, -39 lines 0 comments Download
M utils/pub/yaml/parser.dart View 1 2 20 chunks +107 lines, -169 lines 0 comments Download
M utils/pub/yaml/visitor.dart View 1 2 1 chunk +5 lines, -5 lines 0 comments Download
M utils/pub/yaml/yaml.dart View 1 2 chunks +16 lines, -20 lines 0 comments Download
M utils/pub/yaml/yaml_map.dart View 1 2 5 chunks +16 lines, -22 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Bob Nystrom
I'll do the tests in a separate patch. Yay consistency! Note that this patch includes ...
8 years ago (2012-12-19 01:21:58 UTC) #1
Jennifer Messerly
lgtm
8 years ago (2012-12-19 01:30:48 UTC) #2
nweiz
A couple nits, otherwise LGTM. https://codereview.chromium.org/11638010/diff/2001/tools/line_doc_comments.dart File tools/line_doc_comments.dart (right): https://codereview.chromium.org/11638010/diff/2001/tools/line_doc_comments.dart#newcode20 tools/line_doc_comments.dart:20: print('Usage: ./line_doc_coments.dart <dir>'); Style ...
8 years ago (2012-12-19 20:56:07 UTC) #3
Bob Nystrom
8 years ago (2012-12-20 02:04:15 UTC) #4 
Thanks!

https://codereview.chromium.org/11638010/diff/2001/tools/line_doc_comments.dart
File tools/line_doc_comments.dart (right):

https://codereview.chromium.org/11638010/diff/2001/tools/line_doc_comments.da...
tools/line_doc_comments.dart:20: print('Usage: ./line_doc_coments.dart <dir>');
On 2012/12/19 20:56:07, nweiz wrote:
> Style nit: I'd ditch the "./" here.

Done.

https://codereview.chromium.org/11638010/diff/2001/tools/line_doc_comments.da...
tools/line_doc_comments.dart:33: new
File(path).readAsLines().transform(fixContents).chain((fixed) {
On 2012/12/19 20:56:07, nweiz wrote:
> Maybe a little cleaner to assign the file to a local var?

Done.

https://codereview.chromium.org/11638010/diff/2001/tools/line_doc_comments.da...
tools/line_doc_comments.dart:48: if (match != null) {
On 2012/12/19 20:56:07, nweiz wrote:
> Slightly cleaner to do "if (endBlock.hasMatch(line))", since you never use any
> of the match metadata.

Done.

https://codereview.chromium.org/11638010/diff/2001/tools/line_doc_comments.da...
tools/line_doc_comments.dart:55: line = '$indent/// ${match[1]}';
On 2012/12/19 20:56:07, nweiz wrote:
> This will leave trailing whitespace if `match[1]` is empty.

Done.

https://codereview.chromium.org/11638010/diff/2001/tools/line_doc_comments.da...
tools/line_doc_comments.dart:85: match(RegExp regex, String string,
callback(List<String> groups)) {
On 2012/12/19 20:56:07, nweiz wrote:
> Unused function.

Done.

https://codereview.chromium.org/11638010/diff/2001/utils/pub/entrypoint.dart
File utils/pub/entrypoint.dart (right):

https://codereview.chromium.org/11638010/diff/2001/utils/pub/entrypoint.dart#...
utils/pub/entrypoint.dart:54: // TODO(rnystrom): Make this path configurable.
On 2012/12/19 20:56:07, nweiz wrote:
> Move this above the doc comment.

Done.

https://codereview.chromium.org/11638010/diff/2001/utils/pub/hosted_source.dart
File utils/pub/hosted_source.dart (right):

https://codereview.chromium.org/11638010/diff/2001/utils/pub/hosted_source.da...
utils/pub/hosted_source.dart:107: ///
On 2012/12/19 20:56:07, nweiz wrote:
> Trailing whitespace.

Done.

https://codereview.chromium.org/11638010/diff/2001/utils/pub/hosted_source.da...
utils/pub/hosted_source.dart:137: ///
On 2012/12/19 20:56:07, nweiz wrote:
> More trailing whitespace.

Done.

https://codereview.chromium.org/11638010/diff/2001/utils/pub/root_source.dart
File utils/pub/root_source.dart (right):

https://codereview.chromium.org/11638010/diff/2001/utils/pub/root_source.dart...
utils/pub/root_source.dart:13: ///
On 2012/12/19 20:56:07, nweiz wrote:
> Trailing whitespace.

Done.

Powered by Google App Engine
This is Rietveld 408576698