I don't like writing "Cherry-picks: not for 2.x" in commit messages and then subsequently doing the cherry-pick. That's sending a confusing message in the commit message. We can re-word the sigils if we want to say something about "don't cherry-pick automatically but I'll do it myself" but that just seems to delay the inevitable, and, all things equal, I'd rather the ordering be consistent.
Is the problem the "unnoticed" part of your original e-mail? I'm starting to make it possible for jenkins.impala.io to send e-mails, so it'll be possible for it to send e-mails to the committer of the first broken change, nudging them along. Would that help? Should we stop automatically cherry-picking everything? Now that 3.0 and 2.12 are (nearly/already) released, should we switch to explicit cherry-picking? I'd also be quite happy to start experimenting with review bots that comment on things like "is this a clean cherry-pick" on reviews automatically. -- Philip On Thu, May 3, 2018 at 12:35 PM, Tim Armstrong <tarmstr...@cloudera.com> wrote: > Maybe we should just start with posting back a warning to the code review > if the change doesn't cleanly apply? > > On Thu, May 3, 2018 at 12:34 PM, Tim Armstrong <tarmstr...@cloudera.com> > wrote: > > > It would be good to encourage people to proactively do cherry-picks but I > > think it needs a bit more thought before automating it or adding more > > barriers. > > > > That could potentially cause hold-ups if there's a dependency on a > > previous patch. E.g. if I'm merging two patches B and C that depend on > each > > other and B gets merged, then the state while B is being tested is: > > > > Master > > B->A->... > > > > 2.x > > A->... > > > > and B is in the process of being tested, then what you really want is for > > a new patch to cleanly apply on top of B, but I think your proposed check > > is that it applies on top of A. > > > > It's also introduces the possibility of more mistakes if we encourage > > people to do the cherry-pick to 2.x before precommit tests for master > have > > passed, since they may be cherry-picking a version that doesn't pass > those > > tests. People can of course repeat the cherry-picking process each time > > they push out a new revision but it does introduce more risk of > > accidentally diverging your master and 2.x commits versus just doing the > > conflict resolution once. > > > > On Thu, May 3, 2018 at 12:17 PM, Lars Volker <l...@cloudera.com> wrote: > > > >> Should we add a check to the master GVO that makes sure that a change > >> either applies to 2.x, or has a "not for 2.x" label? > >> > >> On Thu, May 3, 2018 at 12:12 PM, Sailesh Mukil <sail...@cloudera.com> > >> wrote: > >> > >> > It's been a while since we started maintaining 2 active branches, > master > >> > and 2.x, and the divergence between them has grown quite a bit. > >> > > >> > The implications of this (as Michael Brown brought to my attention) > are > >> > that a patch that goes into 'master' may not cleanly apply to '2.x', > >> which > >> > would fail the cherrypick-2.x-and-test( > >> > https://jenkins.impala.io/job/cherrypick-2.x-and-test/) job, and if > >> > unnoticed, can hold up all future cherry-picks from 'master' to '2.x' > >> until > >> > this is resolved. > >> > > >> > If this does happen, the original author of the patch needs to > manually > >> > cherry-pick the patch from 'master' to '2.x', resolve the conflicts > and > >> > push a change for review to the '2.x' branch with the same Change-Id. > >> > > >> > To avoid these types of failures, I advise authors to locally cherry > >> pick > >> > their patch to '2.x' to see if it applies cleanly before running GVO. > >> > > >> > Lastly, if a patch is meant only for 'master', and not for '2.x', > please > >> > remember to add the "Cherry-picks: not for 2.x" line to your commit > >> > message. There have been a few patches in which this was not included, > >> > which unfortunately requires manual work to resolve later on. > >> > > >> > Thank you for your time! > >> > > >> > - Sailesh > >> > > >> > > > > >
