Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1706993002:
Add the @protected annotation (Closed)
|
Created:
4 years, 10 months ago by Brian Wilkerson Modified:
4 years, 10 months ago CC:
reviews_dartlang.org Base URL:
https://github.com/dart-lang/sdk.git@master Target Ref:
refs/heads/master Visibility:
Public. |
DescriptionAdd the @protected annotation
R=ianh@google.com, pquitslund@google.com
Committed: https://github.com/dart-lang/sdk/commit/8df095e6e0f647288841c472d04f29b9607a26ed
Patch Set 1 #
Total comments: 3
Patch Set 2 : address comments #Messages
Total messages: 13 (2 generated)
brianwilkerson@google.com changed reviewers: + ianh@google.com, pquitslund@google.com, rnystrom@google.com
Let me know if we need / want more discussion before starting to implement this first annotation.
Besides ye olde comment style, LGTM!
https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart File pkg/meta/lib/meta.dart (right): https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart#newc... pkg/meta/lib/meta.dart:35: */ I understand if analyzer uses /** */ for historical reasons, but this is a separate package and should really follow the style guide. Who knows, you may even eventually start liking "///". :)
https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart File pkg/meta/lib/meta.dart (right): https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart#newc... pkg/meta/lib/meta.dart:35: */ Any chance we can jump to the future and adopt `///`-style comments?
On 2016/02/17 19:35:08, Bob Nystrom wrote: > https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart > File pkg/meta/lib/meta.dart (right): > > https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart#newc... > pkg/meta/lib/meta.dart:35: */ > I understand if analyzer uses /** */ for historical reasons, but this is a > separate package and should really follow the style guide. > > Who knows, you may even eventually start liking "///". :) Ha. Passed in mid-air. +1! :)
On 2016/02/17 at 19:36:18, pquitslund wrote: > On 2016/02/17 19:35:08, Bob Nystrom wrote: > > https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart > > File pkg/meta/lib/meta.dart (right): > > > > https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart#newc... > > pkg/meta/lib/meta.dart:35: */ > > I understand if analyzer uses /** */ for historical reasons, but this is a > > separate package and should really follow the style guide. > > > > Who knows, you may even eventually start liking "///". :) > > Ha. Passed in mid-air. > > +1! :) LGTM. Does this mean any package that uses @protected has to import 'package:meta/meta.dart'? (I guess in Flutter we can import it somewhere and reexport the ones we care about.)
On 2016/02/17 19:42:25, Hixie wrote: > On 2016/02/17 at 19:36:18, pquitslund wrote: > > On 2016/02/17 19:35:08, Bob Nystrom wrote: > > > https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart > > > File pkg/meta/lib/meta.dart (right): > > > > > > > https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart#newc... > > > pkg/meta/lib/meta.dart:35: */ > > > I understand if analyzer uses /** */ for historical reasons, but this is a > > > separate package and should really follow the style guide. > > > > > > Who knows, you may even eventually start liking "///". :) > > > > Ha. Passed in mid-air. > > > > +1! :) > > LGTM. > > Does this mean any package that uses @protected has to import > 'package:meta/meta.dart'? (I guess in Flutter we can import it somewhere and > reexport the ones we care about.) Yep. Better than depending on analyzer though!
https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart File pkg/meta/lib/meta.dart (right): https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart#newc... pkg/meta/lib/meta.dart:35: */ As much as I prefer "/**" style comments, I was resigned to the need to bow to the prevailing winds. Then I noticed that the file already used "/**" comments for the library doc comment, so I went with the flow. Was I wrong to preserve the already established style?
On 2016/02/17 20:53:19, Brian Wilkerson wrote: > https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart > File pkg/meta/lib/meta.dart (right): > > Was I wrong to preserve the already established style? You have committed no moral crime, but given that the entire meta package contains just two comments, it's not unreasonable to move them both to the preferred style. :)
On 2016/02/17 20:53:19, Brian Wilkerson wrote: > https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart > File pkg/meta/lib/meta.dart (right): > > https://codereview.chromium.org/1706993002/diff/1/pkg/meta/lib/meta.dart#newc... > pkg/meta/lib/meta.dart:35: */ > As much as I prefer "/**" style comments, I was resigned to the need to bow to > the prevailing winds. Then I noticed that the file already used "/**" comments > for the library doc comment, so I went with the flow. Was I wrong to preserve > the already established style? To be clear, this bit of provenance was my fault. I brought `meta` over nearly intact from it's earlier incarnation. What's funny is that I teetered and at the last minute decided NOT to update the comments. Would that I'd gone with my preference and just done it in the first place! :)
Description was changed from ========== Add the @protected annotation ========== to ========== Add the @protected annotation R=ianh@google.com, pquitslund@google.com Committed: https://github.com/dart-lang/sdk/commit/8df095e6e0f647288841c472d04f29b9607a26ed ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 8df095e6e0f647288841c472d04f29b9607a26ed (presubmit successful). |
