Re: Please stop cherry-picking (breaking) changes to the released branches

[Xiangying Meng <xi...@apache.org>]

>Maybe it is better to start a discussion on the mailing list when you want
to
>cherry-pick something and wait for some time.
>If nobody objects then it is good to go

Because there are a large number of PRs that need to be cherry-picked,
some PRs may not strictly abide by this agreement when cherry-picking.
But from the perspective of a release manager,
I think there are three points that we should abide by.
1. As Enrico said in the discussion of starting release 2.10.3,
 it is better not to include the new last-minute things in the release for
a stable branch.
2. The release manager will send an email to notify everyone after all the
PRs with the release label are cherry-picked.
 If there is a new PR that needs to enter this release at this time,
 it is better to submit a PR to cherry-pick it and leave a message under
the mail.
3. If the newly entered commit is verified, whether it runs a CI in its own
repository or in pulsar's repository,
 then we can add the `Verified` label to it.
 Otherwise, it is best not to add the `Verified` label to this commit.

For example, this commit [1] is a new thing that was just merged before
starting this release.
It has not been verified but has a verified label.
Due to my mistake, after noticing the Verified label, I didn't check
further to see if the commit was verified.
This commit introduced some failure in CI, so we now need to re-release.

Of course, this is just a small problem so far.
Fortunately, there is a new change [2] that needs to be imported to
branch-2.10 today,
so we discovered this problem in time.
After all, the current verification process for the release candidates
cannot discover these problems.

Normally, such PRs with minor changes and little conflicts to branch-x can
be directly cherry-picked without running CI,
but it is best not to do this before releasing immediately.

Let`s do our best to make the Pulsar community more mature and perfect.


Sincerely
Xiangying

[1]
https://github.com/apache/pulsar/commit/a3a242cc48e8d7454ee0c5c8fd872ae6f92ae4f7
[2]  https://github.com/apache/pulsar/pull/19711

On Tue, Feb 28, 2023 at 7:17 PM Enrico Olivelli <eo...@gmail.com> wrote:

> Il giorno mar 28 feb 2023 alle ore 12:11 PengHui Li
> <pe...@apache.org> ha scritto:
> >
> > > By the way the main point in this email thread is that we should
> > totally stop to do cherry-picks of stuff that it is
> > not strictly needed
> >
> > Yes, the main issue we need to resolve is how we can define if
> >  the stuff strictly needed to cherry-pick. Do you think the author
> > to provide the cherry-pick information or reviewers to add labels
> > and confirm the label is correct before merging it is a good way?
> > Who wants to update the release/* label, the context is required.
> > Do not only change the release label without any information.
>
> Maybe it is better to start a discussion on the mailing list when you want
> to
> cherry-pick something and wait for some time.
> If nobody objects then it is good to go
>
> Like we slowed down the pace to add new big changes with PIP
> we could follow a slower workflow for cherry-picks.
>
> We could try this strategy for a while.
>
> After all, we are never in a hurry to merge a patch (urgent patches,
> for security issues follow a different path),
> we aren't cutting releases very often.
>
>
> Enrico
>
> >
> > Or, push PR for every cherry-pick to get approved by committers.
> >
> > Thanks,
> > Penghui
> >
> > On Tue, Feb 28, 2023 at 6:32 PM Enrico Olivelli <eo...@gmail.com>
> wrote:
> >
> > > Il giorno mar 28 feb 2023 alle ore 11:19 Yubiao Feng
> > > <yu...@streamnative.io.invalid> ha scritto:
> > > >
> > > > Append asuggestion:
> > > > - After a PR revert, we need to remove the label named "release-xxx",
> > > which
> > > > can alleviate the release manager's work
> > >
> > > I think that it is up to the committer who merges the patch to
> > > cherry-pick immediately to the other branches.
> > > At that point you have enough context to merge the patch and for sure
> > > the committer knows the patch well.
> > >
> > > In Apache BookKeeper and in Apache ZooKeeper we have a script that
> > > does the merge against the target branch and
> > > then it allows you to cherry-pick the other branches.
> > >
> > > Delaying the merge too much makes things harder.
> > >
> > > By the way the main point in this email thread is that we should
> > > totally stop to do cherry-picks of stuff that it is
> > > not strictly needed
> > >
> > >
> > > Enrico
> > >
> > > >
> > > > Thanks
> > > > Yubiao Feng
> > > >
> > > > On Mon, Feb 27, 2023 at 11:27 PM Enrico Olivelli <
> eolivelli@gmail.com>
> > > > wrote:
> > > >
> > > > > Hello Committers,
> > > > > I believe that we should stop cherry-picking breaking changes like
> [1]
> > > > > to released branches.
> > > > > Really, this is something that we cannot do.
> > > > >
> > > > > When you decide to cherry-pick a commit to a "stable branch",
> > > > > currently branch-2.8, branch-2.9, branch-2.10 and branch-2.11 you
> > > > > always have to think about these things:
> > > > > - is it a breaking change ?
> > > > > - is it really needed ?
> > > > > - could it mine the stability of the branch ?
> > > > >
> > > > > The answer is usually that you can cherry-pick a change only if it
> > > > > falls into these categories:
> > > > > - there is a security hole to fix (in this case the PMC has to deal
> > > > > with it, and usually this is done not publicly)
> > > > > - there is a bad bug that cause data loss or other serious problems
> > > > >
> > > > > I have sent this message a few other times in the past.
> > > > > We, the Pulsar community, are responsible for the stability of the
> > > > > project and product that our users use in production.
> > > > >
> > > > > Even if you think that something that could "improve the
> performance"
> > > > > or "do something better" is appealing you always have to keep in
> mind
> > > > > that the risk of breaking something that is stable is too high in
> > > > > respect to the gain in terms of performances or anything else.
> > > > >
> > > > > Improvements should go only to the master branch, and users will
> > > > > benefit from them when we will cut a release.
> > > > >
> > > > > This is a free OSS project on which many users count on.
> > > > >
> > > > > If you are eager to see a performance improvement in your system,
> then
> > > > > this is fine,
> > > > > this is OSS and you can legally have a fork and cherry-pick the
> > > > > patches and build it on your own.
> > > > > This is the reason why OSS is cool.
> > > > > But if you are able to cherry-pick a patch you are also able to
> > > > > maintain your fork and fix any problems if the patch caused a
> > > > > regression.
> > > > >
> > > > > Most of the consumers of OSS products rely on us because they don't
> > > > > have enough engineering resources to maintain such a project by
> > > > > themselves.
> > > > >
> > > > > They trust us and they won't scan a list of tens of commits in
> order
> > > > > to double check if the upgrade will change the behaviour of their
> > > > > applications.
> > > > >
> > > > > This is Pulsar momentum, let's do our best to fulfill the
> expectations
> > > > > of the companies that are adopting our project.
> > > > >
> > > > > Enrico
> > > > >
> > > > > [1]
> > > > >
> > >
> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
> > > > >
> > >
>

Re: Please stop cherry-picking (breaking) changes to the released branches

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

If we are concerned about correctly ensuring only bug fixes go to
release branches, I think we ought to discuss using a merge git
strategy instead of a cherry pick git strategy. This change was
discussed briefly on this mailing list at the end of 2021 [0]. I will
make a brief argument for making the switch, in case there is
interest.

Ultimately, I think this is related to a project's priorities. Do we
prioritize stability on release branches or velocity on the master
branch? Given that we're moving to the LTS plan with 3.0.0, it could
be a good time to make the switch.

Note that one of the primary rebuttals for this feature is that GitHub
defaults PRs to target master. I don't think that is a real problem
for us, though. Clear documentation and a welcoming community can help
new contributors unfamiliar with git understand the workflow.

The primary issue we would face is split brain on our process where
2.x cherry picks bug fixes while 3.0 merges them forward.

For those unfamiliar with the merge strategy: the general flow is that
a bug fix targets the branch where the bug was introduced (or the
oldest active branch) and then we merge each release branch into the
next oldest release branch until we reach master. The benefits include
always running tests before merging/committing changes, more code
reviews for changes being made to release branches, and a clear sense
of where a commit will end up. Also, the git history for a given
commit will show which branches have the commit, which can remove the
need for descriptive (but potentially misleading) labels on PRs.

Finally, the merge strategy should make it easier to quickly cut patch
releases because branches will alway be "ready". That is a huge
benefit for patching security vulnerabilities.

Thanks,
Michael

[0] https://lists.apache.org/thread/zqdqz4jd641vszkj3mzdn6zc3yt56rsk


On Sun, Mar 5, 2023 at 8:52 AM Xiangying Meng <xi...@apache.org> wrote:
>
> >Maybe it is better to start a discussion on the mailing list when you want
> to
> >cherry-pick something and wait for some time.
> >If nobody objects then it is good to go
>
> Because there are a large number of PRs that need to be cherry-picked,
> some PRs may not strictly abide by this agreement when cherry-picking.
> But from the perspective of a release manager,
> I think there are three points that we should abide by.
> 1. As Enrico said in the discussion of starting release 2.10.3,
>  it is better not to include the new last-minute things in the release for
> a stable branch.
> 2. The release manager will send an email to notify everyone after all the
> PRs with the release label are cherry-picked.
>  If there is a new PR that needs to enter this release at this time,
>  it is better to submit a PR to cherry-pick it and leave a message under
> the mail.
> 3. If the newly entered commit is verified, whether it runs a CI in its own
> repository or in pulsar's repository,
>  then we can add the `Verified` label to it.
>  Otherwise, it is best not to add the `Verified` label to this commit.
>
> For example, this commit [1] is a new thing that was just merged before
> starting this release.
> It has not been verified but has a verified label.
> Due to my mistake, after noticing the Verified label, I didn't check
> further to see if the commit was verified.
> This commit introduced some failure in CI, so we now need to re-release.
>
> Of course, this is just a small problem so far.
> Fortunately, there is a new change [2] that needs to be imported to
> branch-2.10 today,
> so we discovered this problem in time.
> After all, the current verification process for the release candidates
> cannot discover these problems.
>
> Normally, such PRs with minor changes and little conflicts to branch-x can
> be directly cherry-picked without running CI,
> but it is best not to do this before releasing immediately.
>
> Let`s do our best to make the Pulsar community more mature and perfect.
>
>
> Sincerely
> Xiangying
>
> [1]
> https://github.com/apache/pulsar/commit/a3a242cc48e8d7454ee0c5c8fd872ae6f92ae4f7
> [2]  https://github.com/apache/pulsar/pull/19711
>
> On Tue, Feb 28, 2023 at 7:17 PM Enrico Olivelli <eo...@gmail.com> wrote:
>
> > Il giorno mar 28 feb 2023 alle ore 12:11 PengHui Li
> > <pe...@apache.org> ha scritto:
> > >
> > > > By the way the main point in this email thread is that we should
> > > totally stop to do cherry-picks of stuff that it is
> > > not strictly needed
> > >
> > > Yes, the main issue we need to resolve is how we can define if
> > >  the stuff strictly needed to cherry-pick. Do you think the author
> > > to provide the cherry-pick information or reviewers to add labels
> > > and confirm the label is correct before merging it is a good way?
> > > Who wants to update the release/* label, the context is required.
> > > Do not only change the release label without any information.
> >
> > Maybe it is better to start a discussion on the mailing list when you want
> > to
> > cherry-pick something and wait for some time.
> > If nobody objects then it is good to go
> >
> > Like we slowed down the pace to add new big changes with PIP
> > we could follow a slower workflow for cherry-picks.
> >
> > We could try this strategy for a while.
> >
> > After all, we are never in a hurry to merge a patch (urgent patches,
> > for security issues follow a different path),
> > we aren't cutting releases very often.
> >
> >
> > Enrico
> >
> > >
> > > Or, push PR for every cherry-pick to get approved by committers.
> > >
> > > Thanks,
> > > Penghui
> > >
> > > On Tue, Feb 28, 2023 at 6:32 PM Enrico Olivelli <eo...@gmail.com>
> > wrote:
> > >
> > > > Il giorno mar 28 feb 2023 alle ore 11:19 Yubiao Feng
> > > > <yu...@streamnative.io.invalid> ha scritto:
> > > > >
> > > > > Append asuggestion:
> > > > > - After a PR revert, we need to remove the label named "release-xxx",
> > > > which
> > > > > can alleviate the release manager's work
> > > >
> > > > I think that it is up to the committer who merges the patch to
> > > > cherry-pick immediately to the other branches.
> > > > At that point you have enough context to merge the patch and for sure
> > > > the committer knows the patch well.
> > > >
> > > > In Apache BookKeeper and in Apache ZooKeeper we have a script that
> > > > does the merge against the target branch and
> > > > then it allows you to cherry-pick the other branches.
> > > >
> > > > Delaying the merge too much makes things harder.
> > > >
> > > > By the way the main point in this email thread is that we should
> > > > totally stop to do cherry-picks of stuff that it is
> > > > not strictly needed
> > > >
> > > >
> > > > Enrico
> > > >
> > > > >
> > > > > Thanks
> > > > > Yubiao Feng
> > > > >
> > > > > On Mon, Feb 27, 2023 at 11:27 PM Enrico Olivelli <
> > eolivelli@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hello Committers,
> > > > > > I believe that we should stop cherry-picking breaking changes like
> > [1]
> > > > > > to released branches.
> > > > > > Really, this is something that we cannot do.
> > > > > >
> > > > > > When you decide to cherry-pick a commit to a "stable branch",
> > > > > > currently branch-2.8, branch-2.9, branch-2.10 and branch-2.11 you
> > > > > > always have to think about these things:
> > > > > > - is it a breaking change ?
> > > > > > - is it really needed ?
> > > > > > - could it mine the stability of the branch ?
> > > > > >
> > > > > > The answer is usually that you can cherry-pick a change only if it
> > > > > > falls into these categories:
> > > > > > - there is a security hole to fix (in this case the PMC has to deal
> > > > > > with it, and usually this is done not publicly)
> > > > > > - there is a bad bug that cause data loss or other serious problems
> > > > > >
> > > > > > I have sent this message a few other times in the past.
> > > > > > We, the Pulsar community, are responsible for the stability of the
> > > > > > project and product that our users use in production.
> > > > > >
> > > > > > Even if you think that something that could "improve the
> > performance"
> > > > > > or "do something better" is appealing you always have to keep in
> > mind
> > > > > > that the risk of breaking something that is stable is too high in
> > > > > > respect to the gain in terms of performances or anything else.
> > > > > >
> > > > > > Improvements should go only to the master branch, and users will
> > > > > > benefit from them when we will cut a release.
> > > > > >
> > > > > > This is a free OSS project on which many users count on.
> > > > > >
> > > > > > If you are eager to see a performance improvement in your system,
> > then
> > > > > > this is fine,
> > > > > > this is OSS and you can legally have a fork and cherry-pick the
> > > > > > patches and build it on your own.
> > > > > > This is the reason why OSS is cool.
> > > > > > But if you are able to cherry-pick a patch you are also able to
> > > > > > maintain your fork and fix any problems if the patch caused a
> > > > > > regression.
> > > > > >
> > > > > > Most of the consumers of OSS products rely on us because they don't
> > > > > > have enough engineering resources to maintain such a project by
> > > > > > themselves.
> > > > > >
> > > > > > They trust us and they won't scan a list of tens of commits in
> > order
> > > > > > to double check if the upgrade will change the behaviour of their
> > > > > > applications.
> > > > > >
> > > > > > This is Pulsar momentum, let's do our best to fulfill the
> > expectations
> > > > > > of the companies that are adopting our project.
> > > > > >
> > > > > > Enrico
> > > > > >
> > > > > > [1]
> > > > > >
> > > >
> > https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
> > > > > >
> > > >
> >