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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2223463003: Switch Dart_Initialize to use a struct (Closed)

Created:
4 years, 4 months ago by abarth-chromium
Modified:
4 years, 4 months ago
Reviewers:
abarth, Cutch, siva, zra
CC:
reviews_dartlang.org, terry, vm-dev_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Switch Dart_Initialize to use a struct The number of NULL parameters was getting out of hand. R=zra@google.com Committed: https://github.com/dart-lang/sdk/commit/bdc25d5695ea913870a44f530ec1a09f363c2e71

Patch Set 1 #

Total comments: 1

Patch Set 2 : More { } #

Total comments: 2

Patch Set 3 : Fix typos #

Patch Set 4 : More typos #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -82 lines) Patch
M runtime/bin/fuchsia_test.cc View 1 2 1 chunk +5 lines, -10 lines 0 comments Download
M runtime/bin/gen_snapshot.cc View 1 2 3 1 chunk +14 lines, -15 lines 0 comments Download
M runtime/bin/main.cc View 1 2 1 chunk +16 lines, -10 lines 0 comments Download
M runtime/include/dart_api.h View 1 2 1 chunk +32 lines, -19 lines 0 comments Download
M runtime/vm/dart_api_impl.cc View 1 2 1 chunk +20 lines, -28 lines 0 comments Download

Messages

Total messages: 13 (3 generated)
abarth
4 years, 4 months ago (2016-08-05 16:14:41 UTC) #2
zra
lgtm with nit but wait for other reviewers https://codereview.chromium.org/2223463003/diff/1/runtime/bin/gen_snapshot.cc File runtime/bin/gen_snapshot.cc (right): https://codereview.chromium.org/2223463003/diff/1/runtime/bin/gen_snapshot.cc#newcode1238 runtime/bin/gen_snapshot.cc:1238: if ...
4 years, 4 months ago (2016-08-05 16:21:30 UTC) #4
abarth
Done. How do I send this change to trybots?
4 years, 4 months ago (2016-08-05 16:30:40 UTC) #5
zra
On 2016/08/05 16:30:40, abarth wrote: > Done. > > How do I send this change ...
4 years, 4 months ago (2016-08-05 16:38:41 UTC) #6
zra
+terry Terry, this is a breaking API change. If it is landed on Monday would ...
4 years, 4 months ago (2016-08-05 16:52:54 UTC) #7
siva
https://codereview.chromium.org/2223463003/diff/20001/runtime/bin/gen_snapshot.cc File runtime/bin/gen_snapshot.cc (right): https://codereview.chromium.org/2223463003/diff/20001/runtime/bin/gen_snapshot.cc#newcode1247 runtime/bin/gen_snapshot.cc:1247: char* error = Dart_Initialize(&flags); Dart_Initialize(&init_params);
4 years, 4 months ago (2016-08-05 17:02:23 UTC) #8
abarth
https://codereview.chromium.org/2223463003/diff/20001/runtime/bin/gen_snapshot.cc File runtime/bin/gen_snapshot.cc (right): https://codereview.chromium.org/2223463003/diff/20001/runtime/bin/gen_snapshot.cc#newcode1247 runtime/bin/gen_snapshot.cc:1247: char* error = Dart_Initialize(&flags); On 2016/08/05 at 17:02:23, siva ...
4 years, 4 months ago (2016-08-05 17:06:23 UTC) #9
zra
I patched in this change locally. It builds and tests pass.
4 years, 4 months ago (2016-08-05 22:30:48 UTC) #10
abarth
Thanks!
4 years, 4 months ago (2016-08-05 22:33:54 UTC) #11
abarth-chromium
4 years, 4 months ago (2016-08-09 19:20:21 UTC) #13
Message was sent while issue was closed. 
Committed patchset #4 (id:60001) manually as
bdc25d5695ea913870a44f530ec1a09f363c2e71 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698