[DISCUSS] Add definition to our cherry picking process

[Michael Marshall <mm...@apache.org>]

Hello Pulsar Community,

As far as I can tell, we do not have a publicly defined process for
cherry-picking commits to release branches [0]. As a new committer,
I'd like to provide my newly gained perspective and ask that we add
some guidance to our wiki.

Regarding cherry picking, I see two patterns in Pulsar:
1. Some committers cherry pick commits when they merge a PR.
2. Some committers leave PRs to be cherry picked right before tagging
the dot release. The release manager is expected to cherry pick these
commits.

Anecdotally, pattern 2 is more common.

I am concerned that pattern 2 creates a lot of extra work for the
release manager, leaves the community with little time to build and
test release branches before tagging release candidates, and leaves us
unable to quickly respond to security issues.

If we switch to pattern 1, merging PRs may become more time consuming.
However, it lightens the load on release managers and makes the
project more nimble. Further, resolving conflicts should be easier, as
the committer already has the PR's context, which could reduce the
risk of regressions.

For example, the 2.7.4 release is delayed because of pattern 2.
Penghui and I cherry picked many commits over the recent days because
branch-2.7 was not up to date. Now, the tests aren't passing for the
branch, and since we shouldn't cut a release until the tests pass, we
haven't released the patch for the Log4Shell CVE.

Another solution is to remove the expectation that the release manager
cherry picks commits. The downside here is that we will possibly miss
important bug fixes that would improve dot release stability.

I propose that we try to use pattern 1 (or something close to it) for
our process.

Thanks,
Michael

[0] https://github.com/apache/pulsar/wiki/Committer-Guide

Re: [DISCUSS] Add definition to our cherry picking process

[Michael Marshall <mm...@apache.org>]

I am very interested in Jonathan's proposal. I think we could really
benefit from a workflow that keeps branches in a (near) ready to
release state and prioritizes the stability of our active branches.

It would be great to hear from other community members regarding our
current process, its benefits and challenges, and potential
alternatives.

For what it's worth, I would be happy to help transition our process
to a merge based workflow.

Thanks,
Michael



On Wed, Dec 15, 2021 at 4:54 PM Jonathan Ellis <jb...@gmail.com> wrote:
>
> First, it's often not an issue -- new contributors typically start with
> small patches that the reviewer or committer can easily rebase to the
> appropriate branch, and by the time they're working on more invasive fixes
> (we don't have any of those, right? :) they get used to how it works.
>
> But when the merge is NOT trivial then the reviewer will ask the
> contributor to post separate patches against the newer branches, this
> reduces the burden on the reviewer (or the release manager!) to do all the
> merging and it is less error prone for the original author to perform a
> tricky merge.
>
> Recent example from Cassandra where reviewer Joey Lynch asked contributor
> NV Harikrishna to do this:
> https://issues.apache.org/jira/browse/CASSANDRA-14898
>
>
>
> On Wed, Dec 15, 2021 at 3:21 PM Enrico Olivelli <eo...@gmail.com> wrote:
>
> > Jonathan,
> >
> > Il Mer 15 Dic 2021, 21:11 Jonathan Ellis <jb...@gmail.com> ha scritto:
> >
> > > What are the benefits of cherry picking vs committing to the oldest
> > > relevant branch and merging forwards?
> >
> >
> > We (and most of the ASF projects I am involved in) ask contributors to
> > target the master branch for the patches.
> >
> > How can we do what you suggest?
> > Asking people to send PRs against the oldest branch relevant to the patch?
> >
> > Probably the contributor is not aware of the other branches.
> >
> > Can you please explain more your idea?
> >
> > Enrico
> >
> > Git is designed around merge since
> > > that preserves the commit sha which allows it to be tracked across
> > > branches.  And a merge based workflow avoids the problem here by design.
> > >
> > > On Wed, Dec 15, 2021 at 1:54 PM Michael Marshall <mm...@apache.org>
> > > wrote:
> > >
> > > > Hello Pulsar Community,
> > > >
> > > > As far as I can tell, we do not have a publicly defined process for
> > > > cherry-picking commits to release branches [0]. As a new committer,
> > > > I'd like to provide my newly gained perspective and ask that we add
> > > > some guidance to our wiki.
> > > >
> > > > Regarding cherry picking, I see two patterns in Pulsar:
> > > > 1. Some committers cherry pick commits when they merge a PR.
> > > > 2. Some committers leave PRs to be cherry picked right before tagging
> > > > the dot release. The release manager is expected to cherry pick these
> > > > commits.
> > > >
> > > > Anecdotally, pattern 2 is more common.
> > > >
> > > > I am concerned that pattern 2 creates a lot of extra work for the
> > > > release manager, leaves the community with little time to build and
> > > > test release branches before tagging release candidates, and leaves us
> > > > unable to quickly respond to security issues.
> > > >
> > > > If we switch to pattern 1, merging PRs may become more time consuming.
> > > > However, it lightens the load on release managers and makes the
> > > > project more nimble. Further, resolving conflicts should be easier, as
> > > > the committer already has the PR's context, which could reduce the
> > > > risk of regressions.
> > > >
> > > > For example, the 2.7.4 release is delayed because of pattern 2.
> > > > Penghui and I cherry picked many commits over the recent days because
> > > > branch-2.7 was not up to date. Now, the tests aren't passing for the
> > > > branch, and since we shouldn't cut a release until the tests pass, we
> > > > haven't released the patch for the Log4Shell CVE.
> > > >
> > > > Another solution is to remove the expectation that the release manager
> > > > cherry picks commits. The downside here is that we will possibly miss
> > > > important bug fixes that would improve dot release stability.
> > > >
> > > > I propose that we try to use pattern 1 (or something close to it) for
> > > > our process.
> > > >
> > > > Thanks,
> > > > Michael
> > > >
> > > > [0] https://github.com/apache/pulsar/wiki/Committer-Guide
> > > >
> > >
> > >
> > > --
> > > Jonathan Ellis
> > > co-founder, http://www.datastax.com
> > > @spyced
> > >
> >
>
>
> --
> Jonathan Ellis
> co-founder, http://www.datastax.com
> @spyced

Re: [DISCUSS] Add definition to our cherry picking process

[Jonathan Ellis <jb...@gmail.com>]

First, it's often not an issue -- new contributors typically start with
small patches that the reviewer or committer can easily rebase to the
appropriate branch, and by the time they're working on more invasive fixes
(we don't have any of those, right? :) they get used to how it works.

But when the merge is NOT trivial then the reviewer will ask the
contributor to post separate patches against the newer branches, this
reduces the burden on the reviewer (or the release manager!) to do all the
merging and it is less error prone for the original author to perform a
tricky merge.

Recent example from Cassandra where reviewer Joey Lynch asked contributor
NV Harikrishna to do this:
https://issues.apache.org/jira/browse/CASSANDRA-14898



On Wed, Dec 15, 2021 at 3:21 PM Enrico Olivelli <eo...@gmail.com> wrote:

> Jonathan,
>
> Il Mer 15 Dic 2021, 21:11 Jonathan Ellis <jb...@gmail.com> ha scritto:
>
> > What are the benefits of cherry picking vs committing to the oldest
> > relevant branch and merging forwards?
>
>
> We (and most of the ASF projects I am involved in) ask contributors to
> target the master branch for the patches.
>
> How can we do what you suggest?
> Asking people to send PRs against the oldest branch relevant to the patch?
>
> Probably the contributor is not aware of the other branches.
>
> Can you please explain more your idea?
>
> Enrico
>
> Git is designed around merge since
> > that preserves the commit sha which allows it to be tracked across
> > branches.  And a merge based workflow avoids the problem here by design.
> >
> > On Wed, Dec 15, 2021 at 1:54 PM Michael Marshall <mm...@apache.org>
> > wrote:
> >
> > > Hello Pulsar Community,
> > >
> > > As far as I can tell, we do not have a publicly defined process for
> > > cherry-picking commits to release branches [0]. As a new committer,
> > > I'd like to provide my newly gained perspective and ask that we add
> > > some guidance to our wiki.
> > >
> > > Regarding cherry picking, I see two patterns in Pulsar:
> > > 1. Some committers cherry pick commits when they merge a PR.
> > > 2. Some committers leave PRs to be cherry picked right before tagging
> > > the dot release. The release manager is expected to cherry pick these
> > > commits.
> > >
> > > Anecdotally, pattern 2 is more common.
> > >
> > > I am concerned that pattern 2 creates a lot of extra work for the
> > > release manager, leaves the community with little time to build and
> > > test release branches before tagging release candidates, and leaves us
> > > unable to quickly respond to security issues.
> > >
> > > If we switch to pattern 1, merging PRs may become more time consuming.
> > > However, it lightens the load on release managers and makes the
> > > project more nimble. Further, resolving conflicts should be easier, as
> > > the committer already has the PR's context, which could reduce the
> > > risk of regressions.
> > >
> > > For example, the 2.7.4 release is delayed because of pattern 2.
> > > Penghui and I cherry picked many commits over the recent days because
> > > branch-2.7 was not up to date. Now, the tests aren't passing for the
> > > branch, and since we shouldn't cut a release until the tests pass, we
> > > haven't released the patch for the Log4Shell CVE.
> > >
> > > Another solution is to remove the expectation that the release manager
> > > cherry picks commits. The downside here is that we will possibly miss
> > > important bug fixes that would improve dot release stability.
> > >
> > > I propose that we try to use pattern 1 (or something close to it) for
> > > our process.
> > >
> > > Thanks,
> > > Michael
> > >
> > > [0] https://github.com/apache/pulsar/wiki/Committer-Guide
> > >
> >
> >
> > --
> > Jonathan Ellis
> > co-founder, http://www.datastax.com
> > @spyced
> >
>


-- 
Jonathan Ellis
co-founder, http://www.datastax.com
@spyced

Re: [DISCUSS] Add definition to our cherry picking process

[Enrico Olivelli <eo...@gmail.com>]

Jonathan,

Il Mer 15 Dic 2021, 21:11 Jonathan Ellis <jb...@gmail.com> ha scritto:

> What are the benefits of cherry picking vs committing to the oldest
> relevant branch and merging forwards?


We (and most of the ASF projects I am involved in) ask contributors to
target the master branch for the patches.

How can we do what you suggest?
Asking people to send PRs against the oldest branch relevant to the patch?

Probably the contributor is not aware of the other branches.

Can you please explain more your idea?

Enrico

Git is designed around merge since
> that preserves the commit sha which allows it to be tracked across
> branches.  And a merge based workflow avoids the problem here by design.
>
> On Wed, Dec 15, 2021 at 1:54 PM Michael Marshall <mm...@apache.org>
> wrote:
>
> > Hello Pulsar Community,
> >
> > As far as I can tell, we do not have a publicly defined process for
> > cherry-picking commits to release branches [0]. As a new committer,
> > I'd like to provide my newly gained perspective and ask that we add
> > some guidance to our wiki.
> >
> > Regarding cherry picking, I see two patterns in Pulsar:
> > 1. Some committers cherry pick commits when they merge a PR.
> > 2. Some committers leave PRs to be cherry picked right before tagging
> > the dot release. The release manager is expected to cherry pick these
> > commits.
> >
> > Anecdotally, pattern 2 is more common.
> >
> > I am concerned that pattern 2 creates a lot of extra work for the
> > release manager, leaves the community with little time to build and
> > test release branches before tagging release candidates, and leaves us
> > unable to quickly respond to security issues.
> >
> > If we switch to pattern 1, merging PRs may become more time consuming.
> > However, it lightens the load on release managers and makes the
> > project more nimble. Further, resolving conflicts should be easier, as
> > the committer already has the PR's context, which could reduce the
> > risk of regressions.
> >
> > For example, the 2.7.4 release is delayed because of pattern 2.
> > Penghui and I cherry picked many commits over the recent days because
> > branch-2.7 was not up to date. Now, the tests aren't passing for the
> > branch, and since we shouldn't cut a release until the tests pass, we
> > haven't released the patch for the Log4Shell CVE.
> >
> > Another solution is to remove the expectation that the release manager
> > cherry picks commits. The downside here is that we will possibly miss
> > important bug fixes that would improve dot release stability.
> >
> > I propose that we try to use pattern 1 (or something close to it) for
> > our process.
> >
> > Thanks,
> > Michael
> >
> > [0] https://github.com/apache/pulsar/wiki/Committer-Guide
> >
>
>
> --
> Jonathan Ellis
> co-founder, http://www.datastax.com
> @spyced
>

Re: [DISCUSS] Add definition to our cherry picking process

[Jonathan Ellis <jb...@gmail.com>]

What are the benefits of cherry picking vs committing to the oldest
relevant branch and merging forwards?  Git is designed around merge since
that preserves the commit sha which allows it to be tracked across
branches.  And a merge based workflow avoids the problem here by design.

On Wed, Dec 15, 2021 at 1:54 PM Michael Marshall <mm...@apache.org>
wrote:

> Hello Pulsar Community,
>
> As far as I can tell, we do not have a publicly defined process for
> cherry-picking commits to release branches [0]. As a new committer,
> I'd like to provide my newly gained perspective and ask that we add
> some guidance to our wiki.
>
> Regarding cherry picking, I see two patterns in Pulsar:
> 1. Some committers cherry pick commits when they merge a PR.
> 2. Some committers leave PRs to be cherry picked right before tagging
> the dot release. The release manager is expected to cherry pick these
> commits.
>
> Anecdotally, pattern 2 is more common.
>
> I am concerned that pattern 2 creates a lot of extra work for the
> release manager, leaves the community with little time to build and
> test release branches before tagging release candidates, and leaves us
> unable to quickly respond to security issues.
>
> If we switch to pattern 1, merging PRs may become more time consuming.
> However, it lightens the load on release managers and makes the
> project more nimble. Further, resolving conflicts should be easier, as
> the committer already has the PR's context, which could reduce the
> risk of regressions.
>
> For example, the 2.7.4 release is delayed because of pattern 2.
> Penghui and I cherry picked many commits over the recent days because
> branch-2.7 was not up to date. Now, the tests aren't passing for the
> branch, and since we shouldn't cut a release until the tests pass, we
> haven't released the patch for the Log4Shell CVE.
>
> Another solution is to remove the expectation that the release manager
> cherry picks commits. The downside here is that we will possibly miss
> important bug fixes that would improve dot release stability.
>
> I propose that we try to use pattern 1 (or something close to it) for
> our process.
>
> Thanks,
> Michael
>
> [0] https://github.com/apache/pulsar/wiki/Committer-Guide
>


-- 
Jonathan Ellis
co-founder, http://www.datastax.com
@spyced