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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 566743002: Don't try to load unnecessary transformers when precompiling executables. (Closed)

Created:
6 years, 3 months ago by nweiz
Modified:
6 years, 3 months ago
CC:
reviews_dartlang.org, Bob Nystrom
Visibility:
Public.

Description

Don't try to load unnecessary transformers when precompiling executables. R=jmesserly@google.com BUG=20917 Committed: https://code.google.com/p/dart/source/detail?r=40179

Patch Set 1 #

Total comments: 2

Messages

Total messages: 6 (1 generated)
nweiz
6 years, 3 months ago (2014-09-11 20:10:54 UTC) #1
Jennifer Messerly
lgtm
6 years, 3 months ago (2014-09-11 20:25:31 UTC) #2
nweiz
Committed patchset #1 (id:1) manually as 40179 (presubmit successful).
6 years, 3 months ago (2014-09-11 21:02:58 UTC) #3
Bob Nystrom
One question but LGTM. https://codereview.chromium.org/566743002/diff/1/sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart File sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart (right): https://codereview.chromium.org/566743002/diff/1/sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart#newcode36 sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart:36: packages: environment.packages); Is this needed? ...
6 years, 3 months ago (2014-09-12 17:57:50 UTC) #5
nweiz
6 years, 3 months ago (2014-09-15 23:17:35 UTC) #6
Message was sent while issue was closed. 
https://codereview.chromium.org/566743002/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart (right):

https://codereview.chromium.org/566743002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart:36: packages:
environment.packages);
On 2014/09/12 17:57:50, Bob Nystrom wrote:
> Is this needed? Won't packages: default to this if omitted?

No, it will default to [environment.graph.packages.keys], which includes
packages that don't need to be loaded.

Powered by Google App Engine
This is Rietveld 408576698