Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2998593002:
Separate the dependencies in the gyp file and use conditional inclusion. (Closed)
DescriptionSeparate the dependencies in the gyp file and use conditional inclusion.
Modified behavior tests to work for 64 bit.
BUG=
Review-Url: https://codereview.chromium.org/2998593002
Committed: https://github.com/google/syzygy/commit/71bfe25d50372c465df18e2818e056dfe8fc9e0a
Patch Set 1 : Separate the dependencies in the gyp file and use conditional inclusion. #Patch Set 2 : Separate the dependencies in the gyp file and use conditional inclusion. #
Total comments: 4
Patch Set 3 : Response to reviewer's comments. #Patch Set 4 : Response to reviewer comments. #Messages
Total messages: 34 (28 generated)
Patchset #1 (id:1) has been deleted
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
njanevsk@google.com changed reviewers: + chrisha@chromium.org, sebmarchand@chromium.org
PTAL.
Patchset #1 (id:20001) has been deleted
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2998593002/diff/60001/syzygy/integration_test... File syzygy/integration_tests/integration_tests.gyp (right): https://codereview.chromium.org/2998593002/diff/60001/syzygy/integration_test... syzygy/integration_tests/integration_tests.gyp:45: 'integration_tests_32bit_dependencies': [ No need to put this in a global variable as it's only used at one place, just put it there and skip using this variable. https://codereview.chromium.org/2998593002/diff/60001/syzygy/integration_test... syzygy/integration_tests/integration_tests.gyp:244: 'integration_tests_clang_dll', You can't add this until this target compile successfully, otherwise it'll break the x64 build, also we want to use the Clang DLL both in x86 and in x64: - For x86 we run the tests on Syzygy + Clang - For x64 we only use Clang.
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL https://codereview.chromium.org/2998593002/diff/60001/syzygy/integration_test... File syzygy/integration_tests/integration_tests.gyp (right): https://codereview.chromium.org/2998593002/diff/60001/syzygy/integration_test... syzygy/integration_tests/integration_tests.gyp:45: 'integration_tests_32bit_dependencies': [ On 2017/08/08 15:01:03, Sébastien Marchand wrote: > No need to put this in a global variable as it's only used at one place, just > put it there and skip using this variable. Done. https://codereview.chromium.org/2998593002/diff/60001/syzygy/integration_test... syzygy/integration_tests/integration_tests.gyp:244: 'integration_tests_clang_dll', On 2017/08/08 15:01:03, Sébastien Marchand wrote: > You can't add this until this target compile successfully, otherwise it'll break > the x64 build, also we want to use the Clang DLL both in x86 and in x64: > - For x86 we run the tests on Syzygy + Clang > - For x64 we only use Clang. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by sebmarchand@chromium.org
lgtm
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1502391419584540,
"parent_rev": "3c81c059f6e414ce979c5654fd88699f8835e89b", "commit_rev":
"71bfe25d50372c465df18e2818e056dfe8fc9e0a"}
Message was sent while issue was closed.
Description was changed from ========== Separate the dependencies in the gyp file and use conditional inclusion. Modified behavior tests to work for 64 bit. BUG= ========== to ========== Separate the dependencies in the gyp file and use conditional inclusion. Modified behavior tests to work for 64 bit. BUG= Review-Url: https://codereview.chromium.org/2998593002 Committed: https://github.com/google/syzygy/commit/71bfe25d50372c465df18e2818e056dfe8fc9e0a ==========
Message was sent while issue was closed.
Committed patchset #4 (id:100001) as https://github.com/google/syzygy/commit/71bfe25d50372c465df18e2818e056dfe8fc9e0a |
