|
|
Created:
4 years, 9 months ago by Michael Achenbach Modified:
4 years, 8 months ago CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionRaise minimum Mac OS version to 10.7
BUG=v8:4847
LOG=y
Committed: https://crrev.com/d298743460a35bc09cb5d6739683a088e0f45190
Cr-Commit-Position: refs/heads/master@{#35148}
Patch Set 1 #Patch Set 2 : 10.7 #Messages
Total messages: 24 (9 generated)
Description was changed from ========== Raise minimum Mac OS version for building to 10.6 BUG= ========== to ========== Raise minimum Mac OS version to 10.7 for building BUG=v8:4847 LOG=y ==========
The CQ bit was checked by machenbach@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/1811283002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1811283002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
machenbach@chromium.org changed reviewers: + dcheng@chromium.org, jkummerow@chromium.org, jochen@chromium.org
PTAL
On 2016/03/18 at 08:52:48, machenbach wrote: > PTAL That's not for building, but for running v8, no?
On 2016/03/18 09:05:38, jochen wrote: > On 2016/03/18 at 08:52:48, machenbach wrote: > > PTAL > > That's not for building, but for running v8, no? You're right. I'm not entirely sure if we need to enforce 10.7, as for chromium I find 10.6 for non-ios here: https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&... wdyt?
Description was changed from ========== Raise minimum Mac OS version to 10.7 for building BUG=v8:4847 LOG=y ========== to ========== Raise minimum Mac OS version to 10.7 BUG=v8:4847 LOG=y ==========
> You're right. I'm not entirely sure if we need to enforce 10.7, as for chromium > I find 10.6 for non-ios here: > https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&... > wdyt? The reason for selecting 10.7 was that it's the minimum required for C++11 support. According to Nico, "In Chromium, we jumped through some hoops to link against libc++ and still target 10.6 for a few releases so we could use C++11 stuff earlier." So it's easier for V8 to just require 10.7 directly. However we should announce this on v8-dev/v8-users. I'm not sure how much of a warning period we should provide -- announcing now that the 5.1 branch will require 10.7 is hopefully fine. (Most users should be on 10.11 anyway; whoever still runs old operating systems probably doesn't care much about running cutting-edge V8 versions.)
In other words, LGTM :-) But please do send out public announcements -- and maybe wait a couple of days before landing this to see if there's significant protest.
lgtm
lgtm
Any update on landing this? I'm happy to send out a PSA if the thread I started on March 7th isn't sufficient.
On 2016/03/31 01:39:21, dcheng wrote: > Any update on landing this? I'm happy to send out a PSA if the thread I started > on March 7th isn't sufficient. There were no objections on the PSA I sent around two weeks ago, so I'll land.
The CQ bit was checked by machenbach@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1811283002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1811283002/20001
bmeurer@chromium.org changed reviewers: + bmeurer@chromium.org
lgtm
Message was sent while issue was closed.
Description was changed from ========== Raise minimum Mac OS version to 10.7 BUG=v8:4847 LOG=y ========== to ========== Raise minimum Mac OS version to 10.7 BUG=v8:4847 LOG=y ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Raise minimum Mac OS version to 10.7 BUG=v8:4847 LOG=y ========== to ========== Raise minimum Mac OS version to 10.7 BUG=v8:4847 LOG=y Committed: https://crrev.com/d298743460a35bc09cb5d6739683a088e0f45190 Cr-Commit-Position: refs/heads/master@{#35148} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/d298743460a35bc09cb5d6739683a088e0f45190 Cr-Commit-Position: refs/heads/master@{#35148} |