I like your policy suggestion. If there are no conflicts, then we can push from master to 2.x without any tests or reviews. If there are conflicts, then a review AND a GVO test run are required.
We can at least start off with this policy and then change it if it is not working well for some reason. On Fri, Jan 19, 2018 at 1:59 PM, Philip Zeyliger <[email protected]> wrote: > An update here! > > I'm pretty close to pushing the first master-only change (the very exciting > 1-liner at https://gerrit.cloudera.org/#/c/9044/ that bumps versions). > After that, I'll be cherrypicking things into 2.x. > > We need a policy, I think, on reviewing these cherry-picks. The most > heavy-weight would be to do Gerrit and run-tests-before-merge for every > change. The least heavy would be to review only when the cherry-picks > aren't clean. Are people comfortable with the less heavy policy? > > Specifically, I propose that clean cherrypicks from master to 2.x can be > pushed without additional review. > > Thanks! > > -- Philip > > On Wed, Jan 17, 2018 at 9:57 AM, Philip Zeyliger <[email protected]> > wrote: > > > It sounds like we've reached consensus, so I'm starting to maneuver this > > along. Please don't hesitate if you've got concerns. You can follow along > > at https://issues.apache.org/jira/browse/IMPALA-6410. > > > > The first two reviews are out at: > > > > remote: http://gerrit.cloudera.org:8080/9044 Bumping version to 3.0. > > remote: http://gerrit.cloudera.org:8080/9045 IMPALA-6410: Tool to > > cherrypick changes across branches. > > > > I've created the branches on Gerrit and Apache as follows: > > > > # This created the branch on gerrit.cloudera.org > > $ssh -p 29418 [email protected] gerrit create-branch Impala-ASF > > 2.x master > > > > # This created the branch on the Apache git server: > > $git push apache asf-gerrit/2.x:refs/heads/2.x > > Username for 'https://git-wip-us.apache.org': philz > > Password for 'https://[email protected]': > > Total 0 (delta 0), reused 0 (delta 0) > > To https://git-wip-us.apache.org/repos/asf/impala.git > > * [new branch] asf-gerrit/2.x -> 2.x > > > > # Double-check: > > $git-ls-remote apache | grep 2.x; git-ls-remote asf-gerrit | grep 2.x > > 6cc76d72016b8d5672676e8e8a979b0807803ad9 refs/heads/2.x > > 6cc76d72016b8d5672676e8e8a979b0807803ad9 refs/heads/2.x > > > > # push_to_asf learned of the branch "magically" > > $bin/push_to_asf.py > > INFO:root:Fetching from remote 'asf-gerrit'... > > INFO:root:done > > Branch '2.x': up to date > > Branch 'asf-site': up to date > > Branch 'branch-2.10.0': found on Apache but not in gerrit > > Branch 'branch-2.11.0': found on Apache but not in gerrit > > Branch 'branch-2.7.0': found on Apache but not in gerrit > > Branch 'branch-2.8.0': found on Apache but not in gerrit > > Branch 'branch-2.9.0': found on Apache but not in gerrit > > Branch 'hadoop-next': up to date > > Branch 'master': up to date > > > > -- Philip > > > > On Tue, Jan 16, 2018 at 5:29 PM, Alexander Behm <[email protected]> > > wrote: > > > >> +1 > >> > >> On Tue, Jan 16, 2018 at 5:27 PM, Taras Bobrovytsky <[email protected] > > > >> wrote: > >> > >> > +1 > >> > > >> > On Tue, Jan 16, 2018 at 1:34 PM, Lars Volker <[email protected]> wrote: > >> > > >> > > +1 > >> > > > >> > > On Tue, Jan 16, 2018 at 1:29 PM, Philip Zeyliger < > [email protected] > >> > > >> > > wrote: > >> > > > >> > > > Hi folks! > >> > > > > >> > > > It sounds like there haven't been objections to having master be > >> "3.0" > >> > > and > >> > > > introducing a 2.x branch. Would folks be alright if I started > making > >> > > > changes in that direction? > >> > > > > >> > > > Thanks! > >> > > > > >> > > > -- Philip > >> > > > > >> > > > On Fri, Jan 12, 2018 at 4:10 PM, Philip Zeyliger < > >> [email protected]> > >> > > > wrote: > >> > > > > >> > > > > > >> > > > > > >> > > > > On Fri, Jan 12, 2018 at 4:04 PM, Jim Apple < > [email protected]> > >> > > wrote: > >> > > > > > >> > > > >> This makes sense to me. > >> > > > >> > >> > > > >> In this mode, for 2.x-only changes and for changes on 3.0 that > >> don't > >> > > > >> apply cleanly, there will be a manual way to do the step > labelled > >> > "1. > >> > > > >> Cherrypick tool", and that way is the same way we send patches > >> for > >> > > > >> review now, but pushing to HEAD:refs/for/2.x rather than > >> > > > >> HEAD:refs/for/master, yes? > >> > > > >> > >> > > > > > >> > > > > Exactly. So, non-clean cherrypicks or 2.x-only changes go > through > >> > > review > >> > > > > on Gerrit, but we give an implicit review pass to clean > >> cherrypicks. > >> > > > > > >> > > > > We could have the cherrypick tool between gerrit/master and > >> > gerrit/2.x > >> > > do > >> > > > > the cherrypicks and run the tests on Jenkins. Do you think > that's > >> > > > > preferable? > >> > > > > > >> > > > > -- Philip > >> > > > > > >> > > > > > >> > > > > > >> > > > >> > >> > > > >> On Fri, Jan 12, 2018 at 3:57 PM, Philip Zeyliger < > >> > [email protected] > >> > > > > >> > > > >> wrote: > >> > > > >> > Picture: > >> > > > >> > https://gist.github.com/philz/323c8b4cb411dc12eb7231d922c195 > >> > > > >> 1f#file-impala-branch-image-pdf > >> > > > >> > > >> > > > >> > > >> > > > >> > On Fri, Jan 12, 2018 at 3:47 PM, Jim Apple < > >> [email protected]> > >> > > > >> wrote: > >> > > > >> > > >> > > > >> >> Often, this list seems to filter out images. Could you post > it > >> > and > >> > > > >> send a > >> > > > >> >> link? > >> > > > >> >> > >> > > > >> >> Thanks for taking this on, Phil! > >> > > > >> >> > >> > > > >> >> On Fri, Jan 12, 2018 at 3:15 PM, Philip Zeyliger < > >> > > > [email protected]> > >> > > > >> >> wrote: > >> > > > >> >> > >> > > > >> >> > I think most patches go to Gerrit branch 'master', which > >> > happens > >> > > to > >> > > > >> >> > identify itself as 3.0. (Or 3.x?). > >> > > > >> >> > > >> > > > >> >> > Here's a picture: > >> > > > >> >> > > >> > > > >> >> > [image: Inline image 1] > >> > > > >> >> > > >> > > > >> >> > > >> > > > >> >> > With this, every time "cherrypick_and_push_to_asf.py" is > >> run, > >> > it > >> > > > >> would > >> > > > >> >> > first offer to cherrypick changes between master and 2.x. > >> Then, > >> > > it > >> > > > >> would > >> > > > >> >> > offer push those cherrypicks to gerrit/2.x. After that, it > >> > > > continues > >> > > > >> on > >> > > > >> >> as > >> > > > >> >> > before and offers to push changes to ASF. I think this > >> > maintains > >> > > > the > >> > > > >> >> > invariant that pushing to ASF is only done with a human > >> > trigger. > >> > > > (We > >> > > > >> >> could > >> > > > >> >> > also have step 1 be done by a Jenkins robot, since it's > >> between > >> > > > >> Gerrit > >> > > > >> >> and > >> > > > >> >> > Gerrit.) > >> > > > >> >> > > >> > > > >> >> > I looked at the How to Release page, and the main > difference > >> > > would > >> > > > be > >> > > > >> >> > that, for a 2.x release, the $COMMIT_HASH_YOU_CHOSE would > >> come > >> > > from > >> > > > >> the > >> > > > >> >> 2.x > >> > > > >> >> > branch, as would any cherrypicks. > >> > > > >> >> > > >> > > > >> >> > Does this match what you're thinking? > >> > > > >> >> > > >> > > > >> >> > Thanks! > >> > > > >> >> > > >> > > > >> >> > -- Philip > >> > > > >> >> > > >> > > > >> >> > On Fri, Jan 12, 2018 at 11:07 AM, Jim Apple < > >> > > [email protected]> > >> > > > >> >> wrote: > >> > > > >> >> > > >> > > > >> >> >> Which gerrit branch were you thinking most patches would > go > >> > to? > >> > > > >> >> >> > >> > > > >> >> >> If they go to 3.0, then push_to_asf.py would have to be > >> > amended > >> > > to > >> > > > >> >> >> push to gerrit, bypassing code review. I think that's > >> > possible, > >> > > > but > >> > > > >> >> >> I'm not 100%. > >> > > > >> >> >> > >> > > > >> >> >> There is also security to think about, since the > >> > push_to_asf.py > >> > > > >> users > >> > > > >> >> >> can push a few commits at a time, including ones they > >> didn't > >> > > > author > >> > > > >> or > >> > > > >> >> >> review. > >> > > > >> >> >> > >> > > > >> >> >> We'll also want to clarify > >> > > > >> >> >> https://cwiki.apache.org/confluence/display/IMPALA/How+ > >> > > to+Release > >> > > > >> and > >> > > > >> >> >> keep it consistent with the git & gerrit statuses quo. > >> > > > >> >> >> > >> > > > >> >> >> On Fri, Jan 12, 2018 at 10:52 AM, Philip Zeyliger < > >> > > > >> [email protected]> > >> > > > >> >> >> wrote: > >> > > > >> >> >> > Hi! > >> > > > >> >> >> > > >> > > > >> >> >> >> Should we start tagging all candidates with a common > >> label, > >> > > > e.g. > >> > > > >> >> >> > include-in-v3? > >> > > > >> >> >> > > >> > > > >> >> >> > I agree with Lars's suggestion for tagging JIRAs with > >> > > > >> include-in-v3. > >> > > > >> >> >> I've > >> > > > >> >> >> > done so, and the relevant query is > >> > > > >> >> >> > https://issues.apache.org/ > jira/issues/?jql=labels%20%3D% > >> > 20in > >> > > > >> >> >> clude-in-v3%20and%20project%3Dimpala > >> > > > >> >> >> > . > >> > > > >> >> >> > > >> > > > >> >> >> >> What sort of process were you thinking of for the > >> > automation? > >> > > > >> >> >> > > >> > > > >> >> >> > I think amending push_to_asf.py, as you suggest, is a > >> great > >> > > > idea. > >> > > > >> I > >> > > > >> >> >> think > >> > > > >> >> >> > we have a string ("not for 2.x") which can be used in > >> commit > >> > > > >> messages > >> > > > >> >> to > >> > > > >> >> >> > discourage the cherrypick for the changes we want to be > >> > > > exclusive > >> > > > >> >> until > >> > > > >> >> >> we > >> > > > >> >> >> > want to change the defaults in the other direction. > >> (I.e., > >> > > right > >> > > > >> now > >> > > > >> >> the > >> > > > >> >> >> > string is "not for 2.x", but at some point the string > >> may be > >> > > > >> "should > >> > > > >> >> be > >> > > > >> >> >> > cherrypicked to 2.x".) > >> > > > >> >> >> > > >> > > > >> >> >> > I do think that we want to create a gerrit branch to > >> allow > >> > > > >> 2.x-only > >> > > > >> >> >> changes > >> > > > >> >> >> > to be reviewed in the straight-forward fashion. > >> > > > >> >> >> > > >> > > > >> >> >> > -- Philip > >> > > > >> >> >> > > >> > > > >> >> >> > > >> > > > >> >> >> > > >> > > > >> >> >> > On Thu, Jan 11, 2018 at 9:31 AM, Jim Apple < > >> > > > [email protected]> > >> > > > >> >> >> wrote: > >> > > > >> >> >> > > >> > > > >> >> >> >> I'm on-board with all of this. (I also would be OK > >> delaying > >> > > > 3.0, > >> > > > >> if > >> > > > >> >> >> >> that were the consensus). > >> > > > >> >> >> >> > >> > > > >> >> >> >> There is one issue in here I think we should dive > into: > >> > > > >> >> >> >> > >> > > > >> >> >> >> > Both master and 2.x would be active, and, at least > for > >> > the > >> > > > >> >> beginning, > >> > > > >> >> >> >> > changes would automatically be pulled into the 2.x > >> line, > >> > > > unless > >> > > > >> >> >> >> explicitly > >> > > > >> >> >> >> > blacklisted. > >> > > > >> >> >> >> > >> > > > >> >> >> >> What sort of process were you thinking of for the > >> > automation? > >> > > > >> >> >> >> > >> > > > >> >> >> >> Some context, starting from what we all likely already > >> > know: > >> > > > >> >> >> >> > >> > > > >> >> >> >> The bulk of the code review and pre-merge testing > >> results > >> > are > >> > > > >> >> recorded > >> > > > >> >> >> >> in gerrit. Once the pre-merge testing passes, a patch > is > >> > > > >> >> cherry-picked > >> > > > >> >> >> >> to the git repo hosted with gerrit. To get the patch > to > >> the > >> > > > >> Impala > >> > > > >> >> git > >> > > > >> >> >> >> repo hosted by the ASF, bin/push_to_asf.py is run by a > >> > human > >> > > > who > >> > > > >> >> >> >> supplies his or her ASF credentials. That script > copies > >> the > >> > > > >> commit to > >> > > > >> >> >> >> the ASF git repo. > >> > > > >> >> >> >> > >> > > > >> >> >> >> Often, 2-3 commits will pile up in gerrit before some > >> > > committer > >> > > > >> runs > >> > > > >> >> >> >> that script and pushes them to ASF. > >> > > > >> >> >> >> > >> > > > >> >> >> >> We could edit that script (bin/push_to_asf.py) to help > >> with > >> > > the > >> > > > >> >> cherry > >> > > > >> >> >> >> picks, so that each time a commit is made, the > committer > >> > must > >> > > > say > >> > > > >> >> >> >> whether the commit goes in 2.x, 3.0, or both, but the > >> > commits > >> > > > are > >> > > > >> >> >> >> often made by people who didn't author the patches, so > >> they > >> > > may > >> > > > >> not > >> > > > >> >> be > >> > > > >> >> >> >> sure which branch to go in. > >> > > > >> >> >> >> > >> > > > >> >> >> >> Additionally, gerrit code review is intimately tied to > >> the > >> > > git > >> > > > >> repo. > >> > > > >> >> >> >> Gerrit runs a git repo under-the-hood, and I believe > >> that > >> > the > >> > > > >> branch > >> > > > >> >> >> >> on gerrit's git that changes are cherry-picked to > after > >> > > > pre-merge > >> > > > >> >> >> >> testing is identical to the Impala git repo hosted by > >> the > >> > > ASF - > >> > > > >> down > >> > > > >> >> >> >> to the hashes, even. If we think 2.x and 3.0 will > >> diverge > >> > > > enough > >> > > > >> that > >> > > > >> >> >> >> we'll want different code reviews for different > >> branches, > >> > > then > >> > > > we > >> > > > >> >> >> >> might want two different branches on gerrit, too. > >> > > > >> >> >> >> > >> > > > >> >> >> > >> > > > >> >> > > >> > > > >> >> > > >> > > > >> >> > >> > > > >> > >> > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > > > >
