Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2047483002:
Remove unused iOS assembly (Closed)
DescriptionRemove unused iOS assembly
R=davidben@chromium.org
Committed: https://crrev.com/b53e7d12dd94a3cde96fda5c3b9898bebaadf744
Cr-Commit-Position: refs/heads/master@{#398131}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Remove unused. #
Messages
Total messages: 21 (6 generated)
The CQ bit was checked by svaldez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2047483002/1
https://codereview.chromium.org/2047483002/diff/1/third_party/boringssl/BUILD.gn File third_party/boringssl/BUILD.gn (right): https://codereview.chromium.org/2047483002/diff/1/third_party/boringssl/BUILD... third_party/boringssl/BUILD.gn:81: if (is_msan || is_ios) { This doesn't seem to have an equivalent in the other one. We'd just fall through to the else block in all the cases. (I don't have strong feelings whether that's a good thing or a bad thing. I guess it'd depend on how iOS assembly would work in the end.)
On 2016/06/06 18:41:34, davidben wrote: > https://codereview.chromium.org/2047483002/diff/1/third_party/boringssl/BUILD.gn > File third_party/boringssl/BUILD.gn (right): > > https://codereview.chromium.org/2047483002/diff/1/third_party/boringssl/BUILD... > third_party/boringssl/BUILD.gn:81: if (is_msan || is_ios) { > This doesn't seem to have an equivalent in the other one. We'd just fall through > to the else block in all the cases. (I don't have strong feelings whether that's > a good thing or a bad thing. I guess it'd depend on how iOS assembly would work > in the end.) There is no great way to chain the conditionals in the gyp code, so falling through to the final case seems right there.
On 2016/06/06 18:43:38, svaldez wrote: > On 2016/06/06 18:41:34, davidben wrote: > > > https://codereview.chromium.org/2047483002/diff/1/third_party/boringssl/BUILD.gn > > File third_party/boringssl/BUILD.gn (right): > > > > > https://codereview.chromium.org/2047483002/diff/1/third_party/boringssl/BUILD... > > third_party/boringssl/BUILD.gn:81: if (is_msan || is_ios) { > > This doesn't seem to have an equivalent in the other one. We'd just fall > through > > to the else block in all the cases. (I don't have strong feelings whether > that's > > a good thing or a bad thing. I guess it'd depend on how iOS assembly would > work > > in the end.) > > There is no great way to chain the conditionals in the gyp code, so falling > through to the final case seems right there. Oh, I meant that I'm not sure BUILD.gn needs any change or mention of iOS at all. It seems it works just fine as it is. It's just that the gyp code we updated the "else" condition but not one of the "if" branches.
On 2016/06/06 18:47:08, davidben wrote: > On 2016/06/06 18:43:38, svaldez wrote: > > On 2016/06/06 18:41:34, davidben wrote: > > > > > > https://codereview.chromium.org/2047483002/diff/1/third_party/boringssl/BUILD.gn > > > File third_party/boringssl/BUILD.gn (right): > > > > > > > > > https://codereview.chromium.org/2047483002/diff/1/third_party/boringssl/BUILD... > > > third_party/boringssl/BUILD.gn:81: if (is_msan || is_ios) { > > > This doesn't seem to have an equivalent in the other one. We'd just fall > > through > > > to the else block in all the cases. (I don't have strong feelings whether > > that's > > > a good thing or a bad thing. I guess it'd depend on how iOS assembly would > > work > > > in the end.) > > > > There is no great way to chain the conditionals in the gyp code, so falling > > through to the final case seems right there. > > Oh, I meant that I'm not sure BUILD.gn needs any change or mention of iOS at > all. It seems it works just fine as it is. It's just that the gyp code we > updated the "else" condition but not one of the "if" branches. (Modulo I suspect iOS + GN doesn't work at all right now, and perhaps GN won't have the same confusion around current_cpu. But we can figure that out when we actually add iOS assembly.)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
On 2016/06/06 19:46:20, commit-bot: I haz the power wrote: > Dry run: Try jobs failed on following builders: > linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, > https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) Done
The CQ bit was checked by davidben@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2047483002/20001
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
Forgot to lgtm. Derp.
The CQ bit was checked by davidben@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2047483002/20001
(Oh, and I guess ticking the CQ box is normally the uploader not the reviewer in Chromium anyway. Oops. I got used to BoringSSL. Anyway...)
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Remove unused iOS assembly R=davidben@chromium.org ========== to ========== Remove unused iOS assembly R=davidben@chromium.org Committed: https://crrev.com/b53e7d12dd94a3cde96fda5c3b9898bebaadf744 Cr-Commit-Position: refs/heads/master@{#398131} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b53e7d12dd94a3cde96fda5c3b9898bebaadf744 Cr-Commit-Position: refs/heads/master@{#398131} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
