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

Issue 1761423002: Uses analyzer ^0.27.2 rather than a fixed 0.27.1 (Closed)

Created:
4 years, 9 months ago by eernst
Modified:
4 years, 9 months ago
Reviewers:
sigurdm
CC:
eernst+reviews_google.com, reviews_dartlang.org, floitsch
Base URL:
https://github.com/dart-lang/reflectable.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Uses analyzer ^0.27.2 rather than a fixed 0.27.1 The transformer is modified to handle the situation where a constant turns out to be unresolved (where `evaluationResult` returns null), which used to prevent us from using any other version of the analyzer than 0.27.1. The approach is lazy and fine-grained in that resolution of constants in a library is requested just before the invocation of `evaluationResult`, and only if this has not already been done. As an extra bonus, the transformation of the 64 tests in 'test_reflectable' is now completed about 12% faster than previously. About one in seven libraries do not get their constants resolved, which is presumably the main reason for the speedup. Fixes https://github.com/dart-lang/reflectable/issues/54. R=sigurdm@google.com Committed: https://github.com/dart-lang/reflectable/commit/543ad76b62681b9cb2a49c163cbefa34d4b521bd

Patch Set 1 #

Total comments: 4

Patch Set 2 : Review response #

Unified diffs Side-by-side diffs Delta from patch set Stats (+109 lines, -48 lines) Patch
M reflectable/lib/src/transformer_implementation.dart View 1 27 chunks +108 lines, -47 lines 0 comments Download
M reflectable/pubspec.yaml View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (2 generated)
eernst
After 4-5 months where we've been stuck on using 'analyzer' version 0.27.1, we can finally ...
4 years, 9 months ago (2016-03-04 17:36:33 UTC) #2
sigurdm
LGTM https://codereview.chromium.org/1761423002/diff/1/reflectable/lib/src/transformer_implementation.dart File reflectable/lib/src/transformer_implementation.dart (right): https://codereview.chromium.org/1761423002/diff/1/reflectable/lib/src/transformer_implementation.dart#newcode4608 reflectable/lib/src/transformer_implementation.dart:4608: String get documentationComment { Are we using this? ...
4 years, 9 months ago (2016-03-07 08:53:08 UTC) #3
eernst
Review response. Landing now. https://codereview.chromium.org/1761423002/diff/1/reflectable/lib/src/transformer_implementation.dart File reflectable/lib/src/transformer_implementation.dart (right): https://codereview.chromium.org/1761423002/diff/1/reflectable/lib/src/transformer_implementation.dart#newcode4608 reflectable/lib/src/transformer_implementation.dart:4608: String get documentationComment { On ...
4 years, 9 months ago (2016-03-07 14:50:34 UTC) #4
eernst
4 years, 9 months ago (2016-03-07 15:05:11 UTC) #6
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
543ad76b62681b9cb2a49c163cbefa34d4b521bd (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698