CL labelled as Trybot-Ready +1 can't be applied in pre-cq. |
||||
Issue descriptionI have CL stack as follows: crosreview.com/#/c/540269 crosreview.com/#/c/556455 crosreview.com/#/c/539030 crosreview.com/#/c/539029 All CLs were marked Trybot-Ready=+1, but since CL 556455 was new, it had not been reviewed yet. When the pre-cq tried to apply 556455, it borked: "15:25:58: INFO: Failed creating transaction for phobbs:540269:2f55b5c2: CL:540269 depends on CL:556465, which is not marked Code-Review=+2." See the logs here: https://luci-logdog.appspot.com/v/?s=chromeos%2Fbb%2Fchromeos%2Fpre-cq-launcher%2F9475%2F%2B%2Frecipes%2Fsteps%2FPreCQLauncher%2F0%2Fstdout Why does launching a Pre-CQ trybot require Code-Review=+2 on every CL in the stack?
,
Jun 30 2017
Why is this intended?
,
Oct 14 2017
,
May 18 2018
,
Oct 31
Oof, I'll second comment #2. This really doesn't sound like (good) intentional behavior. Anecdote: I'm pretty sure some developers are working around this by just +2'ing their own stuff sometimes (I mean, the PreCQ is almost telling them to...). See this CL (and its CQ-DEPEND): https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1260426 and some of the discussion, particularly: "self reviewed otherwise pre-cq will complain the depended cl not CR+2."
,
Oct 31
PreCQ will be going away as we roll out parallel CQ in Q1. As such, this report will be obsoleted then. As a side effect of rolling out PCQ, all CL's will no longer need to be CR+2 to do a test run. (CQ+1 will be like launching a tryjob/PreCQ; CQ+2 will be for doing a real CQ run.)
,
Oct 31
#6 "As a side effect of rolling out PCQ, all CL's will no longer need to be CR+2 to do a test run." wouldn't this mean anyone with a chromium account can run any arbitrary code on our builders without approval? -cracks knuckles and makes a mine-bitcoin@chromium.org account-
,
Oct 31
We will prevent launching CQ+1 if the CQ+1 flagger doesn't have commit access. That will be orthogonal to code review.
,
Oct 31
#8 Is that how pre-CQ works today? That seems like a decent solution.
,
Oct 31
Today, Pre-CQ only looks at CQ+2 labels. Please add your comments to this DD so we don't lose them: https://docs.google.com/document/d/1AI8xuDHyG1nzuzDgpRjm_jVyDS1cBF1ujsB3Gl49QLM/edit |
||||
►
Sign in to add a comment |
||||
Comment 1 by jrbarnette@chromium.org
, Jun 30 2017Owner: dgarr...@chromium.org
Status: Assigned (was: Untriaged)