Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1860353002:
Remove XP Condition Variable Affordances (Closed)
DescriptionRemove XP Condition Variable Affordances
XP and Vista are no longer supported in Chromium.
BUG=579196
Committed: https://crrev.com/f11a6c0284b264679e9a86d9c0a2624204bba793
Cr-Commit-Position: refs/heads/master@{#385665}
Patch Set 1 : #
Total comments: 3
Patch Set 2 : Change declaration order #
Dependent Patchsets: Messages
Total messages: 36 (15 generated)
Patchset #1 (id:1) has been deleted
The CQ bit was checked by robliao@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/1860353002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1860353002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by robliao@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/1860353002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1860353002/20001
robliao@chromium.org changed reviewers: + danakj@chromium.org
danakj@: Please review this changelist. Thanks!
scottmg FYI.
scottmg@chromium.org changed reviewers: + scottmg@chromium.org
https://codereview.chromium.org/1860353002/diff/20001/base/synchronization/co... File base/synchronization/condition_variable.h (right): https://codereview.chromium.org/1860353002/diff/20001/base/synchronization/co... base/synchronization/condition_variable.h:106: CRITICAL_SECTION* const crit_sec_; nit; swap order to make win & posix match.
Well that's simpler, isn't it. LGTM
https://codereview.chromium.org/1860353002/diff/20001/base/synchronization/co... File base/synchronization/condition_variable.h (right): https://codereview.chromium.org/1860353002/diff/20001/base/synchronization/co... base/synchronization/condition_variable.h:106: CRITICAL_SECTION* const crit_sec_; On 2016/04/06 18:34:04, scottmg wrote: > nit; swap order to make win & posix match. I would have done that, but it diverges from the init order :-/. We get the crit_sec_ via initialization list (since it's const here) and then we make the InitializeConditionVariable call in the constructor body.
https://codereview.chromium.org/1860353002/diff/20001/base/synchronization/co... File base/synchronization/condition_variable.h (right): https://codereview.chromium.org/1860353002/diff/20001/base/synchronization/co... base/synchronization/condition_variable.h:106: CRITICAL_SECTION* const crit_sec_; On 2016/04/06 19:22:18, robliao wrote: > On 2016/04/06 18:34:04, scottmg wrote: > > nit; swap order to make win & posix match. > > I would have done that, but it diverges from the init order :-/. > > We get the crit_sec_ via initialization list (since it's const here) and then we > make the InitializeConditionVariable call in the constructor body. I guess we already do that with user_lock_. Hands up in the air! Done.
The CQ bit was checked by robliao@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org Link to the patchset: https://codereview.chromium.org/1860353002/#ps40001 (title: "Change declaration order")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1860353002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1860353002/40001
The CQ bit was unchecked by robliao@chromium.org
The CQ bit was checked by robliao@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/1860353002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1860353002/40001
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/04/06 23:19:34, 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...) It appears because I changed base, this CL is triggering timeouts. If there are no objections, I'll likely git cl land this tomorrow morning. linux_chromium_asan_rel_ng Start Wed Apr 6 14:30:24 2016 End Wed Apr 6 16:30:25 2016 Elapsed 2 hrs, 0 secs https://code.google.com/p/chromium/codesearch#chromium/build/masters/master.t... # Sanity timeout for CQ builders - they are expected to finish under one hour # anyway. The timeout is deliberately larger than that so that we only # kill really crazy long builds that also gum up resources. CQ_MAX_TIME = 2*60*60 linux_android_rel_ng Start Wed Apr 6 14:22:29 2016 End Wed Apr 6 16:17:29 2016 Elapsed 1 hrs, 55 mins, 0 secs https://code.google.com/p/chromium/codesearch#chromium/build/masters/master.t... "linux_android_rel_ng": { "builder_timeout_s": 6900, # 60 * 115
The CQ bit was checked by robliao@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/1860353002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1860353002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/04/07 01:34:43, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. Looks like I got lucky and got a short build cycle. The CQ says we're good.
The CQ bit was checked by robliao@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1860353002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1860353002/40001
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Remove XP Condition Variable Affordances XP and Vista are no longer supported in Chromium. BUG=579196 ========== to ========== Remove XP Condition Variable Affordances XP and Vista are no longer supported in Chromium. BUG=579196 Committed: https://crrev.com/f11a6c0284b264679e9a86d9c0a2624204bba793 Cr-Commit-Position: refs/heads/master@{#385665} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/f11a6c0284b264679e9a86d9c0a2624204bba793 Cr-Commit-Position: refs/heads/master@{#385665}
Message was sent while issue was closed.
On Wed, Apr 6, 2016 at 4:49 PM, <robliao@chromium.org> wrote: > On 2016/04/06 23:19:34, 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... > ) > > It appears because I changed base, this CL is triggering timeouts. If > there are > no objections, I'll likely git cl land this tomorrow morning. > > linux_chromium_asan_rel_ng > Start Wed Apr 6 14:30:24 2016 > End Wed Apr 6 16:30:25 2016 > Elapsed 2 hrs, 0 secs > > > https://code.google.com/p/chromium/codesearch#chromium/build/masters/master.t... > # Sanity timeout for CQ builders - they are expected to finish under one > hour > # anyway. The timeout is deliberately larger than that so that we only > # kill really crazy long builds that also gum up resources. > CQ_MAX_TIME = 2*60*60 > > linux_android_rel_ng > Start Wed Apr 6 14:22:29 2016 > End Wed Apr 6 16:17:29 2016 > Elapsed 1 hrs, 55 mins, 0 secs > > > https://code.google.com/p/chromium/codesearch#chromium/build/masters/master.t... > "linux_android_rel_ng": { > "builder_timeout_s": 6900, # 60 * 115 > Oh my gosh. Can you please file a bug with this @ the CQ folks? -- 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.
I was going to do that, but I had trouble getting a clean run. (the referenced runs were also victims of a bad tree). Feel free to track with http://crbug.com/601547 On Thu, Apr 7, 2016 at 11:56 AM, Dana Jansens <danakj@chromium.org> wrote: > On Wed, Apr 6, 2016 at 4:49 PM, <robliao@chromium.org> wrote: > >> On 2016/04/06 23:19:34, 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... >> ) >> >> It appears because I changed base, this CL is triggering timeouts. If >> there are >> no objections, I'll likely git cl land this tomorrow morning. >> >> linux_chromium_asan_rel_ng >> Start Wed Apr 6 14:30:24 2016 >> End Wed Apr 6 16:30:25 2016 >> Elapsed 2 hrs, 0 secs >> >> >> https://code.google.com/p/chromium/codesearch#chromium/build/masters/master.t... >> # Sanity timeout for CQ builders - they are expected to finish under one >> hour >> # anyway. The timeout is deliberately larger than that so that we only >> # kill really crazy long builds that also gum up resources. >> CQ_MAX_TIME = 2*60*60 >> >> linux_android_rel_ng >> Start Wed Apr 6 14:22:29 2016 >> End Wed Apr 6 16:17:29 2016 >> Elapsed 1 hrs, 55 mins, 0 secs >> >> >> https://code.google.com/p/chromium/codesearch#chromium/build/masters/master.t... >> "linux_android_rel_ng": { >> "builder_timeout_s": 6900, # 60 * 115 >> > > > Oh my gosh. Can you please file a bug with this @ the CQ folks? > -- 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. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
