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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2924833002: Pass a Target instance to DillTarget instead of its name (Closed)

Created:
3 years, 6 months ago by Dmitry Stefantsov
Modified:
3 years, 6 months ago
Reviewers:
ahe, scheglov
CC:
reviews_dartlang.org, dart-fe-team+reviews_google.com, Kevin Millikin (Google)
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Pass a Target instance to DillTarget instead of its name R=ahe@google.com Committed: https://github.com/dart-lang/sdk/commit/09e755196a51a848426b925f123fbddf2147f6b2

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -42 lines) Patch
M pkg/compiler/lib/src/kernel/fasta_support.dart View 2 chunks +3 lines, -4 lines 0 comments Download
M pkg/front_end/lib/kernel_generator.dart View 3 chunks +5 lines, -6 lines 0 comments Download
M pkg/front_end/lib/src/fasta/compile_platform.dart View 2 chunks +5 lines, -3 lines 0 comments Download
M pkg/front_end/lib/src/fasta/dill/dill_target.dart View 2 chunks +3 lines, -6 lines 0 comments Download
M pkg/front_end/lib/src/fasta/fasta.dart View 4 chunks +10 lines, -8 lines 2 comments Download
M pkg/front_end/lib/src/incremental_kernel_generator_impl.dart View 2 chunks +4 lines, -2 lines 0 comments Download
M pkg/front_end/test/fasta/shaker_test.dart View 2 chunks +4 lines, -2 lines 0 comments Download
M pkg/front_end/test/fasta/testing/suite.dart View 2 chunks +4 lines, -3 lines 0 comments Download
M pkg/front_end/tool/fasta_perf.dart View 2 chunks +3 lines, -2 lines 0 comments Download
M pkg/kernel/test/closures/suite.dart View 2 chunks +4 lines, -3 lines 0 comments Download
M pkg/kernel/test/interpreter/suite.dart View 2 chunks +4 lines, -3 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
Dmitry Stefantsov
3 years, 6 months ago (2017-06-06 13:56:37 UTC) #2
ahe
lgtm https://codereview.chromium.org/2924833002/diff/1/pkg/front_end/lib/src/fasta/fasta.dart File pkg/front_end/lib/src/fasta/fasta.dart (right): https://codereview.chromium.org/2924833002/diff/1/pkg/front_end/lib/src/fasta/fasta.dart#newcode120 pkg/front_end/lib/src/fasta/fasta.dart:120: getTarget(c.options.target, Perhaps we should move this to compiler_options.dart?
3 years, 6 months ago (2017-06-06 14:02:37 UTC) #3
Dmitry Stefantsov
Committed patchset #1 (id:1) manually as 09e755196a51a848426b925f123fbddf2147f6b2 (presubmit successful).
3 years, 6 months ago (2017-06-06 14:03:57 UTC) #5
scheglov
LGTM
3 years, 6 months ago (2017-06-06 14:06:49 UTC) #6
Dmitry Stefantsov
3 years, 6 months ago (2017-06-06 14:13:26 UTC) #7
Message was sent while issue was closed. 
https://codereview.chromium.org/2924833002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/fasta.dart (right):

https://codereview.chromium.org/2924833002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/fasta.dart:120: getTarget(c.options.target,
On 2017/06/06 14:02:37, ahe wrote:
> Perhaps we should move this to compiler_options.dart?

Sorry for missing it again XD I'll be more careful next time.

Do you mean that we can add "getTarget()" to the options object?

Powered by Google App Engine
This is Rietveld 408576698