Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2450933002:
win: Remove unneeded references to visual_studio_version now that it's always 2015. (Closed)
Descriptionwin: Remove unneeded references to visual_studio_version now that it's always 2015.
BUG=
Committed: https://crrev.com/a628db5056deb88589224bd68dcd9ec029932c83
Cr-Commit-Position: refs/heads/master@{#427463}
Patch Set 1 #
Total comments: 2
Patch Set 2 : . #Patch Set 3 : . #Patch Set 4 : . #
Messages
Total messages: 31 (19 generated)
The CQ bit was checked by thakis@chromium.org to run a CQ dry run
thakis@chromium.org changed reviewers: + scottmg@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm, thanks https://codereview.chromium.org/2450933002/diff/1/build/config/win/visual_stu... File build/config/win/visual_studio_version.gni (right): https://codereview.chromium.org/2450933002/diff/1/build/config/win/visual_stu... build/config/win/visual_studio_version.gni:12: # Currently always "2015". I guess we can't just remove it because bots want to set it? https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by thakis@chromium.org 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...
https://codereview.chromium.org/2450933002/diff/1/build/config/win/visual_stu... File build/config/win/visual_studio_version.gni (right): https://codereview.chromium.org/2450933002/diff/1/build/config/win/visual_stu... build/config/win/visual_studio_version.gni:12: # Currently always "2015". On 2016/10/25 19:16:27, scottmg wrote: > I guess we can't just remove it because bots want to set it? > https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium/... Hm, I'm not sure that that file is still used; I think most gn args stuff happens in mb these days and all the gn stuff in recipe_modules is dead code. So maybe I can try removing it in a follow-up.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by thakis@chromium.org 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: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by thakis@chromium.org 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 thakis@chromium.org
The CQ bit was checked by thakis@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org Link to the patchset: https://codereview.chromium.org/2450933002/#ps60001 (title: ".")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== win: Remove unneeded references to visual_studio_version now that it's always 2015. BUG= ========== to ========== win: Remove unneeded references to visual_studio_version now that it's always 2015. BUG= Committed: https://crrev.com/a628db5056deb88589224bd68dcd9ec029932c83 Cr-Commit-Position: refs/heads/master@{#427463} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/a628db5056deb88589224bd68dcd9ec029932c83 Cr-Commit-Position: refs/heads/master@{#427463}
Message was sent while issue was closed.
(I looked at removing this completely, but it looks like other projects deps in these files and read this var :-/)
Message was sent while issue was closed.
On 2016/10/26 14:29:09, Nico wrote: > (I looked at removing this completely, but it looks like other projects deps in > these files and read this var :-/) Darn. Are they also checking for things-that-can't-happen-anymore then? Or do they actually work on 2013? (Maybe file a follow up bug with the details if you feel up to it.)
Message was sent while issue was closed.
On 2016/10/26 15:57:35, scottmg (slow on 25nov) wrote: > On 2016/10/26 14:29:09, Nico wrote: > > (I looked at removing this completely, but it looks like other projects deps > in > > these files and read this var :-/) > > Darn. Are they also checking for things-that-can't-happen-anymore then? Or do > they actually work on 2013? (Maybe file a follow up bug with the details if you > feel up to it.) This wasn't solved yet: http://crbug.com/603131 :( Now our deps roller is blocked on this.
Message was sent while issue was closed.
Revert this and add a comment that v8 relies on it, with link to bug. Maybe there should be a v8 trybot in the default for build/ changes. On Nov 28, 2016 5:33 AM, <machenbach@chromium.org> wrote: > On 2016/10/26 15:57:35, scottmg (slow on 25nov) wrote: > > On 2016/10/26 14:29:09, Nico wrote: > > > (I looked at removing this completely, but it looks like other > projects deps > > in > > > these files and read this var :-/) > > > > Darn. Are they also checking for things-that-can't-happen-anymore then? > Or do > > they actually work on 2013? (Maybe file a follow up bug with the details > if > you > > feel up to it.) > > This wasn't solved yet: http://crbug.com/603131 > > :( > > Now our deps roller is blocked on this. > > https://codereview.chromium.org/2450933002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2531333004/ by machenbach@chromium.org. The reason for reverting is: V8 relies on this..
Message was sent while issue was closed.
On 2016/11/28 13:05:56, Nico wrote:
> Revert this and add a comment that v8 relies on it, with link to bug. Maybe
> there should be a v8 trybot in the default for build/ changes.
Revert in progress. Trybots sound good, but are they feasible? We'd need trybots
for {linux, win, mac, android} X {release, debug} to have reasonable coverage.
Currently the auto-build-deps roller is blocked on three distinct issues
specific to linux, win and mac. Usually I deal with those faster, but due to no
cycles, this time they started piling up.
Though we might need different platforms, I think just running gn and compiling
might be enough. Will think about it.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
