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

Issue 796353003: Incremental compiler detects when library graph changes. (Closed)

Created:
6 years ago by ahe
Modified:
6 years ago
Reviewers:
Johnni Winther
CC:
reviews_dartlang.org, kasperl, lukechurch
Target Ref:
refs/remotes/trunk
Visibility:
Public.

Description

Incremental compiler detects when library graph changes. Also, don't suppress all exceptions as it hid errors in the test. R=johnniwinther@google.com Committed: https://code.google.com/p/dart/source/detail?r=42505

Patch Set 1 #

Patch Set 2 : Merged with r42504. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+109 lines, -20 lines) Patch
M dart/pkg/dart2js_incremental/lib/dart2js_incremental.dart View 1 chunk +8 lines, -0 lines 0 comments Download
M dart/pkg/dart2js_incremental/lib/library_updater.dart View 5 chunks +85 lines, -16 lines 0 comments Download
M dart/tests/try/web/incremental_compilation_update_test.dart View 2 chunks +16 lines, -4 lines 0 comments Download

Messages

Total messages: 8 (1 generated)
ahe
Baby steps: only detection. The incremental compiler magter ikke if you change library, import, export, ...
6 years ago (2014-12-19 09:29:16 UTC) #2
Johnni Winther
Please add some tests. Otherwise LGTM
6 years ago (2014-12-19 12:07:40 UTC) #3
Johnni Winther
Disregard the adding test comment.
6 years ago (2014-12-19 12:35:26 UTC) #4
ahe
On 2014/12/19 12:35:26, Johnni Winther wrote: > Disregard the adding test comment. So I convinced ...
6 years ago (2014-12-19 12:59:46 UTC) #5
ahe
And thank you for taking a look :-)
6 years ago (2014-12-19 13:00:12 UTC) #6
ahe
Committed patchset #2 (id:20001) manually as 42505 (presubmit successful).
6 years ago (2014-12-19 13:13:30 UTC) #7
ahe
6 years ago (2014-12-19 14:02:06 UTC) #8
Message was sent while issue was closed.
On 2014/12/19 12:59:46, ahe wrote:
> On 2014/12/19 12:35:26, Johnni Winther wrote:
> > Disregard the adding test comment.
> 
> So I convinced you in person that it already was covered by the existing test.
> But that isn't exactly true. I need to have a test which adds an import (and,
> for now, ensure it fails).
> 
> I'll do that in the next CL.

The next CL is: https://codereview.chromium.org/818713002

Powered by Google App Engine
This is Rietveld 408576698