CQ: do not merge CLs still marked as Private |
|||
Issue descriptiononce we start to merge a CL, there's no reason for it to be marked Private, or to let it stay that way once it's been merged. this makes it hard for people who aren't directly cc-ed to review the comments. we should update the CQ bots to filter out is:private CLs, and possibly have it post a comment like "Refusing to pick up Private CLs" while removing the Commit-Queue+1 bit. this bug is about the Chrome OS CQ, but i'm wondering whether the Chromium team has a policy on this already. seems like we should be aligned.
,
Mar 26 2018
my only concern is about Merged|Private. if we can disallow that, i'm happy to leave Private usage available in all other cases.
,
Mar 26 2018
...I sincerely believed that [Merged, Private] was an impossible state. I'm clearly wrong: https://chromium-review.googlesource.com/c/v8/v8/+/981232 That's certainly exciting. Yes, I believe it makes sense for all changes to be made non-private at the time they're submitted.
,
Mar 27 2018
i see about 40 or so CLs on our internal repo: https://chrome-internal-review.googlesource.com/q/status:merged+is:private and even more on our public GoB (100s that i have access to): https://chromium-review.googlesource.com/q/status:merged+is:private i'm fairly certain none of those were intentional :/
,
Jun 28 2018
,
Jun 28 2018
On the ChromeOS side, we can allow private for tryjobs (and even the PreCQ) while preventing the CQ from picking up Private CLs. However, if a CL is marked Private while the CQ is running, it will still be possible for it to submitted.
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/f9d85d8779cf4352bb19557064dc3b3ae423fc21 commit f9d85d8779cf4352bb19557064dc3b3ae423fc21 Author: Mike Frysinger <vapier@chromium.org> Date: Tue Jul 10 05:35:42 2018 cbuildbot: reject private CLs in the CQ We don't want to merge CLs marked private in the CQ (pre-cq is fine), so reject ones that are still marked as private. BUG= chromium:825407 TEST=precq passes Change-Id: If09243920c08f2ff505801d43f84353ff2259184 Reviewed-on: https://chromium-review.googlesource.com/1117908 Commit-Ready: Mike Frysinger <vapier@chromium.org> Tested-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/f9d85d8779cf4352bb19557064dc3b3ae423fc21/lib/patch_unittest.py [modify] https://crrev.com/f9d85d8779cf4352bb19557064dc3b3ae423fc21/lib/patch.py
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/25fbb27791f08bdcfb0fa50727d088ca5b5f517b commit 25fbb27791f08bdcfb0fa50727d088ca5b5f517b Author: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Tue Jul 10 06:53:33 2018 Roll src/third_party/chromite 1956af3165b5..f9d85d8779cf (1 commits) https://chromium.googlesource.com/chromiumos/chromite.git/+log/1956af3165b5..f9d85d8779cf git log 1956af3165b5..f9d85d8779cf --date=short --no-merges --format='%ad %ae %s' 2018-07-10 vapier@chromium.org cbuildbot: reject private CLs in the CQ Created with: gclient setdep -r src/third_party/chromite@f9d85d8779cf The AutoRoll server is located here: https://chromite-chromium-roll.skia.org Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. BUG= chromium:825407 TBR=chrome-os-gardeners@chromium.org Change-Id: I39f4073739c6e896156adbf86de9ead6d2b098f4 Reviewed-on: https://chromium-review.googlesource.com/1130682 Reviewed-by: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Commit-Queue: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#573636} [modify] https://crrev.com/25fbb27791f08bdcfb0fa50727d088ca5b5f517b/DEPS
,
Jul 11
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/a3bc29bcacf392cdba0021fee8bab5bd45665975 commit a3bc29bcacf392cdba0021fee8bab5bd45665975 Author: Mike Frysinger <vapier@chromium.org> Date: Wed Jul 11 19:13:32 2018 validation_pool: process private/draft CLs before filtering We're now correctly filtering out private CLs, but we still aren't notifying the user why they're being elided. It's because the validation pool will filter out non-ready CLs first, then only process the ones left. Turns out we've also been silently skipping draft CLs too. Update the validation pool to notify draft/private CLs before we filter out the non-ready CLs. BUG= chromium:825407 TEST=precq passes Change-Id: Ia32f634ae033fef61128d700d056fe4537cd88a1 Reviewed-on: https://chromium-review.googlesource.com/1132709 Commit-Ready: Mike Frysinger <vapier@chromium.org> Tested-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Don Garrett <dgarrett@chromium.org> [modify] https://crrev.com/a3bc29bcacf392cdba0021fee8bab5bd45665975/cbuildbot/validation_pool.py [modify] https://crrev.com/a3bc29bcacf392cdba0021fee8bab5bd45665975/lib/patch.py
,
Jul 11
the CrOS CQ now detects & rejects CLs that are marked Private and CQ+1. example: > https://chromium-review.googlesource.com/1130703 > Patch Set 2: > The Commit Queue could not apply your change because the CL is private. Please make your CL public before marking your commit as ready. so i'm going to call this fixed from the CrOS side. if Chromium wants to update their CQ flow, we can fork a new bug. |
|||
►
Sign in to add a comment |
|||
Comment 1 by aga...@chromium.org
, Mar 26 2018