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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >> 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 >> > >> > >
