You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Enrico Olivelli <eo...@gmail.com> on 2023/02/27 15:26:48 UTC

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

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

Posted by 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
> > > > > >
> > > >
> >

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

Posted by 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

Posted by Enrico Olivelli <eo...@gmail.com>.
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 <eo...@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

Posted by PengHui Li <pe...@apache.org>.
> 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.

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 <eo...@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

Posted by Enrico Olivelli <eo...@gmail.com>.
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 <eo...@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

Posted by Yubiao Feng <yu...@streamnative.io.INVALID>.
Append asuggestion:
- After a PR revert, we need to remove the label named "release-xxx", which
can alleviate the release manager's work

Thanks
Yubiao Feng

On Mon, Feb 27, 2023 at 11:27 PM Enrico Olivelli <eo...@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

Posted by Niclas Hedhman <ni...@hedhman.org>.
Enrico,
I find it interesting that the main reaction is to focus on the symptom, 
rather than the disease.

 From my experience, the open source project should drop the support of 
multiple release branches (especially since they are claimed to be 
backward compatible), only have one released version and only append new 
work on main or develop branch. That also opens up for commercial 
companies to offer long-term support for those companies that think that 
it is too difficult to keep upgrading.



On 2023-02-27 16:26, Enrico Olivelli 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

Posted by ma...@gmail.com.
+1000,

We should pay attention on two places:

1. Why add release/xxx label to the breaking change PR ?
2. We should recheck the context when committer cherry-pick PRs.


Best,
Mattison
On Feb 27, 2023, 23:27 +0800, Enrico Olivelli <eo...@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

Posted by Niclas Hedhman <ni...@hedhman.org>.
Linus Thorvalds has spoken about this more than once, and his position 
is that the ultimate failure of a project is to worsen the user 
experience, and changing behavior is "worse experience". He claim to go 
as far as revert bug fixes if there are people depending on the buggy 
behavior and calls for "alternative ways" to mitigate the bug.

Being a minor Pulsar user, I fully support Enrico's call here.

Cheers
Niclas

On 2023-02-27 16:26, Enrico Olivelli 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

Posted by Zike Yang <zi...@apache.org>.
> If the author haven't provided the context, any committer who want to add
 the release/* label should left a comment about why the PR should be
cherry-pick.

Totally agree. The committer needs to add at least one comment for the
reason if he wants to add the release labels.

I think we could add this to the committer guideline if we reach this consensus.

Thanks,
Zike Yang

On Tue, Feb 28, 2023 at 3:19 PM PengHui Li <pe...@apache.org> wrote:
>
> I found another example https://github.com/apache/pulsar/pull/19425
> Which has cherry-picked and then reverted
>
> If the PR's author knows the PR should be cherry-pick to some branches,
> it's better to contains this part in the PR description and explain why the
> fix
> should be cherry-picked. The reviewer should also review whether it is
> worth cherry-picking and then updating the labels.
>
> If the author haven't provided the context, any committer who want to add
>  the release/* label should left a comment about why the PR should be
> cherry-pick.
>
> I think it will helped the release manager to understand should or
> shouldn't
> to cherry-pick the PRs.
>
> We can also update the PR template
>
> Thanks,
> Penghui
>
>
> On Tue, Feb 28, 2023 at 1:36 PM Dave Fisher <wa...@comcast.net> wrote:
>
> >
> >
> > Sent from my iPhone
> >
> > > On Feb 27, 2023, at 9:28 PM, tison <wa...@gmail.com> wrote:
> > >
> > > 
> > >>
> > >> Yes, but..
> > >
> > > For this case, I agree that the RM trust Jason who tagged the PR needs to
> > > be picked to 2.10. In this case Jason was supposed to do the check.
> > >
> > >> guidance for release managers to evaluate PR cherry-pick labels
> > carefully
> > >
> > > For the current workflow, the RM can trust the labels since only
> > committers
> > > can label a PR.
> >
> > The RM should still verify what they are trusting. People make mistakes.
> > >
> > > We may add some guidelines in both label strategy and release process on
> > > the Contribution Guide to talk about breaking changes. Generally:
> > >
> > > * Default config value.
> > > * Public API breaking.
> > > * Dependency upgrading (may or may not a breaking change)
> > > * Other user-visible behavior changes (in the issued PR, it changes
> > whether
> > > an offload data is deleted on topic deleted)
> > >
> > > There's no automation/tests to cover all the cases so we still rely on
> > > proper reviews and tags.
> > >
> > > Best,
> > > tison.
> > >
> > >
> > > Dave Fisher <wa...@comcast.net> 于2023年2月28日周二 13:21写道:
> > >
> > >>
> > >>
> > >> Sent from my iPhone
> > >>
> > >>>> On Feb 27, 2023, at 9:08 PM, tison <wa...@gmail.com> wrote:
> > >>>
> > >>> 
> > >>>>
> > >>>> The release manager is unable to review all PRs before releasing it.
> > >>>
> > >>> At least the RM is responsible for PRs cherry-picked he/she made. As we
> > >>> take compatibility in a high priority, if it's unclear a fix (patch)
> > >>> without breaking changes, the RM can ask for confirmation.
> > >>
> > >> Is there guidance for release managers to evaluate PR cherry-pick labels
> > >> carefully (how to confirm)?
> > >>
> > >> Best,
> > >> Dave
> > >>>
> > >>> Best,
> > >>> tison.
> > >>>
> > >>>
> > >>> PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:
> > >>>
> > >>>> Hi enrico,
> > >>>>
> > >>>> +1 for your point.
> > >>>>
> > >>>> Do you know the details of the breaking change?
> > >>>> I can't find any discussions under the mailing list about the breaking
> > >>>> change.
> > >>>>
> > >>>> I have added the `release/important-notice ` label to the PR, and we
> > >> should
> > >>>> also discuss first, better to have a proposal if we are making
> > breaking
> > >>>> changes.
> > >>>>
> > >>>> IMO, the main issue here is that the release manager doesn't know the
> > >> PR is
> > >>>> introducing breaking changes, rather than thinking that the
> > >> introduction of
> > >>>> breaking changes is reasonable to the patch release. I noticed Jason
> > had
> > >>>> added the release/* label, I think he also isn't aware of the breaking
> > >>>> change.
> > >>>>
> > >>>> The release manager is unable to review all PRs before releasing it.
> > >>>> And the PR title said
> > >>>>
> > >>>> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
> > >>>> Deletion".
> > >>>>
> > >>>> My impression, it also should be bug fix.
> > >>>>
> > >>>> Regards,
> > >>>> Penghui
> > >>>>
> > >>>>> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <
> > xiangying@apache.org>
> > >>>>> wrote:
> > >>>>>
> > >>>>> Hi Enrico Olivelli,
> > >>>>>
> > >>>>> Totally agree, we should be careful when cherry-picking PRs. And we
> > >> can't
> > >>>>> trust our own judgment too much. For an uncertain PR, we must submit
> > a
> > >> PR
> > >>>>> and wait for everyone to review it together.
> > >>>>> For example, for the PR [1] mentioned above, the measure I took was
> > to
> > >>>> push
> > >>>>> a PR to cherry-pick and move it to the next release version (2.10.5)
> > so
> > >>>>> that we have enough time to discuss and reach an agreement.
> > >>>>>
> > >>>>> Sincerely,
> > >>>>> Xiangying
> > >>>>> [1]
> > >>>>>
> > >>
> > https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
> > >>>>>
> > >>>>> On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
> > >>>>> <yu...@streamnative.io.invalid> wrote:
> > >>>>>
> > >>>>>> Hi Enrico Olivelli
> > >>>>>>
> > >>>>>> Thank you for helping me correct my mistake
> > >>>>>>
> > >>>>>> 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

Posted by PengHui Li <pe...@apache.org>.
I found another example https://github.com/apache/pulsar/pull/19425
Which has cherry-picked and then reverted

If the PR's author knows the PR should be cherry-pick to some branches,
it's better to contains this part in the PR description and explain why the
fix
should be cherry-picked. The reviewer should also review whether it is
worth cherry-picking and then updating the labels.

If the author haven't provided the context, any committer who want to add
 the release/* label should left a comment about why the PR should be
cherry-pick.

I think it will helped the release manager to understand should or
shouldn't
to cherry-pick the PRs.

We can also update the PR template

Thanks,
Penghui


On Tue, Feb 28, 2023 at 1:36 PM Dave Fisher <wa...@comcast.net> wrote:

>
>
> Sent from my iPhone
>
> > On Feb 27, 2023, at 9:28 PM, tison <wa...@gmail.com> wrote:
> >
> > 
> >>
> >> Yes, but..
> >
> > For this case, I agree that the RM trust Jason who tagged the PR needs to
> > be picked to 2.10. In this case Jason was supposed to do the check.
> >
> >> guidance for release managers to evaluate PR cherry-pick labels
> carefully
> >
> > For the current workflow, the RM can trust the labels since only
> committers
> > can label a PR.
>
> The RM should still verify what they are trusting. People make mistakes.
> >
> > We may add some guidelines in both label strategy and release process on
> > the Contribution Guide to talk about breaking changes. Generally:
> >
> > * Default config value.
> > * Public API breaking.
> > * Dependency upgrading (may or may not a breaking change)
> > * Other user-visible behavior changes (in the issued PR, it changes
> whether
> > an offload data is deleted on topic deleted)
> >
> > There's no automation/tests to cover all the cases so we still rely on
> > proper reviews and tags.
> >
> > Best,
> > tison.
> >
> >
> > Dave Fisher <wa...@comcast.net> 于2023年2月28日周二 13:21写道:
> >
> >>
> >>
> >> Sent from my iPhone
> >>
> >>>> On Feb 27, 2023, at 9:08 PM, tison <wa...@gmail.com> wrote:
> >>>
> >>> 
> >>>>
> >>>> The release manager is unable to review all PRs before releasing it.
> >>>
> >>> At least the RM is responsible for PRs cherry-picked he/she made. As we
> >>> take compatibility in a high priority, if it's unclear a fix (patch)
> >>> without breaking changes, the RM can ask for confirmation.
> >>
> >> Is there guidance for release managers to evaluate PR cherry-pick labels
> >> carefully (how to confirm)?
> >>
> >> Best,
> >> Dave
> >>>
> >>> Best,
> >>> tison.
> >>>
> >>>
> >>> PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:
> >>>
> >>>> Hi enrico,
> >>>>
> >>>> +1 for your point.
> >>>>
> >>>> Do you know the details of the breaking change?
> >>>> I can't find any discussions under the mailing list about the breaking
> >>>> change.
> >>>>
> >>>> I have added the `release/important-notice ` label to the PR, and we
> >> should
> >>>> also discuss first, better to have a proposal if we are making
> breaking
> >>>> changes.
> >>>>
> >>>> IMO, the main issue here is that the release manager doesn't know the
> >> PR is
> >>>> introducing breaking changes, rather than thinking that the
> >> introduction of
> >>>> breaking changes is reasonable to the patch release. I noticed Jason
> had
> >>>> added the release/* label, I think he also isn't aware of the breaking
> >>>> change.
> >>>>
> >>>> The release manager is unable to review all PRs before releasing it.
> >>>> And the PR title said
> >>>>
> >>>> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
> >>>> Deletion".
> >>>>
> >>>> My impression, it also should be bug fix.
> >>>>
> >>>> Regards,
> >>>> Penghui
> >>>>
> >>>>> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <
> xiangying@apache.org>
> >>>>> wrote:
> >>>>>
> >>>>> Hi Enrico Olivelli,
> >>>>>
> >>>>> Totally agree, we should be careful when cherry-picking PRs. And we
> >> can't
> >>>>> trust our own judgment too much. For an uncertain PR, we must submit
> a
> >> PR
> >>>>> and wait for everyone to review it together.
> >>>>> For example, for the PR [1] mentioned above, the measure I took was
> to
> >>>> push
> >>>>> a PR to cherry-pick and move it to the next release version (2.10.5)
> so
> >>>>> that we have enough time to discuss and reach an agreement.
> >>>>>
> >>>>> Sincerely,
> >>>>> Xiangying
> >>>>> [1]
> >>>>>
> >>
> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
> >>>>>
> >>>>> On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
> >>>>> <yu...@streamnative.io.invalid> wrote:
> >>>>>
> >>>>>> Hi Enrico Olivelli
> >>>>>>
> >>>>>> Thank you for helping me correct my mistake
> >>>>>>
> >>>>>> 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

Posted by Dave Fisher <wa...@comcast.net>.

Sent from my iPhone

> On Feb 27, 2023, at 9:28 PM, tison <wa...@gmail.com> wrote:
> 
> 
>> 
>> Yes, but..
> 
> For this case, I agree that the RM trust Jason who tagged the PR needs to
> be picked to 2.10. In this case Jason was supposed to do the check.
> 
>> guidance for release managers to evaluate PR cherry-pick labels carefully
> 
> For the current workflow, the RM can trust the labels since only committers
> can label a PR.

The RM should still verify what they are trusting. People make mistakes.
> 
> We may add some guidelines in both label strategy and release process on
> the Contribution Guide to talk about breaking changes. Generally:
> 
> * Default config value.
> * Public API breaking.
> * Dependency upgrading (may or may not a breaking change)
> * Other user-visible behavior changes (in the issued PR, it changes whether
> an offload data is deleted on topic deleted)
> 
> There's no automation/tests to cover all the cases so we still rely on
> proper reviews and tags.
> 
> Best,
> tison.
> 
> 
> Dave Fisher <wa...@comcast.net> 于2023年2月28日周二 13:21写道:
> 
>> 
>> 
>> Sent from my iPhone
>> 
>>>> On Feb 27, 2023, at 9:08 PM, tison <wa...@gmail.com> wrote:
>>> 
>>> 
>>>> 
>>>> The release manager is unable to review all PRs before releasing it.
>>> 
>>> At least the RM is responsible for PRs cherry-picked he/she made. As we
>>> take compatibility in a high priority, if it's unclear a fix (patch)
>>> without breaking changes, the RM can ask for confirmation.
>> 
>> Is there guidance for release managers to evaluate PR cherry-pick labels
>> carefully (how to confirm)?
>> 
>> Best,
>> Dave
>>> 
>>> Best,
>>> tison.
>>> 
>>> 
>>> PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:
>>> 
>>>> Hi enrico,
>>>> 
>>>> +1 for your point.
>>>> 
>>>> Do you know the details of the breaking change?
>>>> I can't find any discussions under the mailing list about the breaking
>>>> change.
>>>> 
>>>> I have added the `release/important-notice ` label to the PR, and we
>> should
>>>> also discuss first, better to have a proposal if we are making breaking
>>>> changes.
>>>> 
>>>> IMO, the main issue here is that the release manager doesn't know the
>> PR is
>>>> introducing breaking changes, rather than thinking that the
>> introduction of
>>>> breaking changes is reasonable to the patch release. I noticed Jason had
>>>> added the release/* label, I think he also isn't aware of the breaking
>>>> change.
>>>> 
>>>> The release manager is unable to review all PRs before releasing it.
>>>> And the PR title said
>>>> 
>>>> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
>>>> Deletion".
>>>> 
>>>> My impression, it also should be bug fix.
>>>> 
>>>> Regards,
>>>> Penghui
>>>> 
>>>>> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <xi...@apache.org>
>>>>> wrote:
>>>>> 
>>>>> Hi Enrico Olivelli,
>>>>> 
>>>>> Totally agree, we should be careful when cherry-picking PRs. And we
>> can't
>>>>> trust our own judgment too much. For an uncertain PR, we must submit a
>> PR
>>>>> and wait for everyone to review it together.
>>>>> For example, for the PR [1] mentioned above, the measure I took was to
>>>> push
>>>>> a PR to cherry-pick and move it to the next release version (2.10.5) so
>>>>> that we have enough time to discuss and reach an agreement.
>>>>> 
>>>>> Sincerely,
>>>>> Xiangying
>>>>> [1]
>>>>> 
>> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
>>>>> 
>>>>> On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
>>>>> <yu...@streamnative.io.invalid> wrote:
>>>>> 
>>>>>> Hi Enrico Olivelli
>>>>>> 
>>>>>> Thank you for helping me correct my mistake
>>>>>> 
>>>>>> 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

Posted by tison <wa...@gmail.com>.
> Yes, but..

For this case, I agree that the RM trust Jason who tagged the PR needs to
be picked to 2.10. In this case Jason was supposed to do the check.

> guidance for release managers to evaluate PR cherry-pick labels carefully

For the current workflow, the RM can trust the labels since only committers
can label a PR.

We may add some guidelines in both label strategy and release process on
the Contribution Guide to talk about breaking changes. Generally:

* Default config value.
* Public API breaking.
* Dependency upgrading (may or may not a breaking change)
* Other user-visible behavior changes (in the issued PR, it changes whether
an offload data is deleted on topic deleted)

There's no automation/tests to cover all the cases so we still rely on
proper reviews and tags.

Best,
tison.


Dave Fisher <wa...@comcast.net> 于2023年2月28日周二 13:21写道:

>
>
> Sent from my iPhone
>
> > On Feb 27, 2023, at 9:08 PM, tison <wa...@gmail.com> wrote:
> >
> > 
> >>
> >> The release manager is unable to review all PRs before releasing it.
> >
> > At least the RM is responsible for PRs cherry-picked he/she made. As we
> > take compatibility in a high priority, if it's unclear a fix (patch)
> > without breaking changes, the RM can ask for confirmation.
>
> Is there guidance for release managers to evaluate PR cherry-pick labels
> carefully (how to confirm)?
>
> Best,
> Dave
> >
> > Best,
> > tison.
> >
> >
> > PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:
> >
> >> Hi enrico,
> >>
> >> +1 for your point.
> >>
> >> Do you know the details of the breaking change?
> >> I can't find any discussions under the mailing list about the breaking
> >> change.
> >>
> >> I have added the `release/important-notice ` label to the PR, and we
> should
> >> also discuss first, better to have a proposal if we are making breaking
> >> changes.
> >>
> >> IMO, the main issue here is that the release manager doesn't know the
> PR is
> >> introducing breaking changes, rather than thinking that the
> introduction of
> >> breaking changes is reasonable to the patch release. I noticed Jason had
> >> added the release/* label, I think he also isn't aware of the breaking
> >> change.
> >>
> >> The release manager is unable to review all PRs before releasing it.
> >> And the PR title said
> >>
> >> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
> >> Deletion".
> >>
> >> My impression, it also should be bug fix.
> >>
> >> Regards,
> >> Penghui
> >>
> >>> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <xi...@apache.org>
> >>> wrote:
> >>>
> >>> Hi Enrico Olivelli,
> >>>
> >>> Totally agree, we should be careful when cherry-picking PRs. And we
> can't
> >>> trust our own judgment too much. For an uncertain PR, we must submit a
> PR
> >>> and wait for everyone to review it together.
> >>> For example, for the PR [1] mentioned above, the measure I took was to
> >> push
> >>> a PR to cherry-pick and move it to the next release version (2.10.5) so
> >>> that we have enough time to discuss and reach an agreement.
> >>>
> >>> Sincerely,
> >>> Xiangying
> >>> [1]
> >>>
> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
> >>>
> >>> On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
> >>> <yu...@streamnative.io.invalid> wrote:
> >>>
> >>>> Hi Enrico Olivelli
> >>>>
> >>>> Thank you for helping me correct my mistake
> >>>>
> >>>> 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

Posted by Dave Fisher <wa...@comcast.net>.

Sent from my iPhone

> On Feb 27, 2023, at 9:08 PM, tison <wa...@gmail.com> wrote:
> 
> 
>> 
>> The release manager is unable to review all PRs before releasing it.
> 
> At least the RM is responsible for PRs cherry-picked he/she made. As we
> take compatibility in a high priority, if it's unclear a fix (patch)
> without breaking changes, the RM can ask for confirmation.

Is there guidance for release managers to evaluate PR cherry-pick labels carefully (how to confirm)?

Best,
Dave
> 
> Best,
> tison.
> 
> 
> PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:
> 
>> Hi enrico,
>> 
>> +1 for your point.
>> 
>> Do you know the details of the breaking change?
>> I can't find any discussions under the mailing list about the breaking
>> change.
>> 
>> I have added the `release/important-notice ` label to the PR, and we should
>> also discuss first, better to have a proposal if we are making breaking
>> changes.
>> 
>> IMO, the main issue here is that the release manager doesn't know the PR is
>> introducing breaking changes, rather than thinking that the introduction of
>> breaking changes is reasonable to the patch release. I noticed Jason had
>> added the release/* label, I think he also isn't aware of the breaking
>> change.
>> 
>> The release manager is unable to review all PRs before releasing it.
>> And the PR title said
>> 
>> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
>> Deletion".
>> 
>> My impression, it also should be bug fix.
>> 
>> Regards,
>> Penghui
>> 
>>> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <xi...@apache.org>
>>> wrote:
>>> 
>>> Hi Enrico Olivelli,
>>> 
>>> Totally agree, we should be careful when cherry-picking PRs. And we can't
>>> trust our own judgment too much. For an uncertain PR, we must submit a PR
>>> and wait for everyone to review it together.
>>> For example, for the PR [1] mentioned above, the measure I took was to
>> push
>>> a PR to cherry-pick and move it to the next release version (2.10.5) so
>>> that we have enough time to discuss and reach an agreement.
>>> 
>>> Sincerely,
>>> Xiangying
>>> [1]
>>> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
>>> 
>>> On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
>>> <yu...@streamnative.io.invalid> wrote:
>>> 
>>>> Hi Enrico Olivelli
>>>> 
>>>> Thank you for helping me correct my mistake
>>>> 
>>>> Yubiao Feng
>>>> 
>>>> On Mon, Feb 27, 2023 at 11:27 PM Enrico Olivelli <eo...@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

Posted by tison <wa...@gmail.com>.
I agree that the `release/*` tags are misleading hints. Remove all the tags
for the issued PR.

Best,
tison.


tison <wa...@gmail.com> 于2023年2月28日周二 13:13写道:

> For "what is breaking changes", at least the PULL_REQUEST_TEMPLATE gives
> some hints:
> https://github.com/apache/pulsar/blob/bbb543d8ed2b03361807c852da1a31cfb92939f3/.github/PULL_REQUEST_TEMPLATE.md?plain=1#L56-L72
>
> For the issued PR, Hang commented at
> https://github.com/apache/pulsar/pull/15914#issuecomment-1146942281 that
> it changes the default behavior.
>
> An alternative is that the original PR author can hint a ! in title or
> BREAKING CHANGE and commit message like conventionalcommits spec defines.
>
> Best,
> tison.
>
>
> tison <wa...@gmail.com> 于2023年2月28日周二 13:07写道:
>
>> > The release manager is unable to review all PRs before releasing it.
>>
>> At least the RM is responsible for PRs cherry-picked he/she made. As we
>> take compatibility in a high priority, if it's unclear a fix (patch)
>> without breaking changes, the RM can ask for confirmation.
>>
>> Best,
>> tison.
>>
>>
>> PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:
>>
>>> Hi enrico,
>>>
>>> +1 for your point.
>>>
>>> Do you know the details of the breaking change?
>>> I can't find any discussions under the mailing list about the breaking
>>> change.
>>>
>>> I have added the `release/important-notice ` label to the PR, and we
>>> should
>>> also discuss first, better to have a proposal if we are making breaking
>>> changes.
>>>
>>> IMO, the main issue here is that the release manager doesn't know the PR
>>> is
>>> introducing breaking changes, rather than thinking that the introduction
>>> of
>>> breaking changes is reasonable to the patch release. I noticed Jason had
>>> added the release/* label, I think he also isn't aware of the breaking
>>> change.
>>>
>>> The release manager is unable to review all PRs before releasing it.
>>> And the PR title said
>>>
>>> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
>>> Deletion".
>>>
>>> My impression, it also should be bug fix.
>>>
>>> Regards,
>>> Penghui
>>>
>>> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <xi...@apache.org>
>>> wrote:
>>>
>>> > Hi Enrico Olivelli,
>>> >
>>> > Totally agree, we should be careful when cherry-picking PRs. And we
>>> can't
>>> > trust our own judgment too much. For an uncertain PR, we must submit a
>>> PR
>>> > and wait for everyone to review it together.
>>> > For example, for the PR [1] mentioned above, the measure I took was to
>>> push
>>> > a PR to cherry-pick and move it to the next release version (2.10.5) so
>>> > that we have enough time to discuss and reach an agreement.
>>> >
>>> > Sincerely,
>>> > Xiangying
>>> > [1]
>>> >
>>> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
>>> >
>>> > On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
>>> > <yu...@streamnative.io.invalid> wrote:
>>> >
>>> > >  Hi Enrico Olivelli
>>> > >
>>> > > Thank you for helping me correct my mistake
>>> > >
>>> > > 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

Posted by PengHui Li <pe...@apache.org>.
> For the issued PR, Hang commented at
https://github.com/apache/pulsar/pull/15914#issuecomment-1146942281 that it
changes the default behavior.

Yes, but it looks like it has been ignored by the reviewers and the author.
The PR still gets merged, finally. We should give a `Change Request` if the
PR
is trying to introduce potential breaking changes.
The committer mergingthe PR might also miss
 the comment (not change request).

Thanks,
Penghui


On Tue, Feb 28, 2023 at 1:18 PM PengHui Li <pe...@apache.org> wrote:

> > At least the RM is responsible for PRs cherry-picked he/she made. As we
> take compatibility in a high priority, if it's unclear a fix (patch)
> without breaking changes, the RM can ask for confirmation.
>
> Yes, but it's not easy without going through all the comments,
> review the PR changes. If we don't have an eye-catching reminder.
>
> Thanks,
> Penghui
>
> On Tue, Feb 28, 2023 at 1:14 PM tison <wa...@gmail.com> wrote:
>
>> For "what is breaking changes", at least the PULL_REQUEST_TEMPLATE gives
>> some hints:
>>
>> https://github.com/apache/pulsar/blob/bbb543d8ed2b03361807c852da1a31cfb92939f3/.github/PULL_REQUEST_TEMPLATE.md?plain=1#L56-L72
>>
>> For the issued PR, Hang commented at
>> https://github.com/apache/pulsar/pull/15914#issuecomment-1146942281 that
>> it
>> changes the default behavior.
>>
>> An alternative is that the original PR author can hint a ! in title or
>> BREAKING CHANGE and commit message like conventionalcommits spec defines.
>>
>> Best,
>> tison.
>>
>>
>> tison <wa...@gmail.com> 于2023年2月28日周二 13:07写道:
>>
>> > > The release manager is unable to review all PRs before releasing it.
>> >
>> > At least the RM is responsible for PRs cherry-picked he/she made. As we
>> > take compatibility in a high priority, if it's unclear a fix (patch)
>> > without breaking changes, the RM can ask for confirmation.
>> >
>> > Best,
>> > tison.
>> >
>> >
>> > PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:
>> >
>> >> Hi enrico,
>> >>
>> >> +1 for your point.
>> >>
>> >> Do you know the details of the breaking change?
>> >> I can't find any discussions under the mailing list about the breaking
>> >> change.
>> >>
>> >> I have added the `release/important-notice ` label to the PR, and we
>> >> should
>> >> also discuss first, better to have a proposal if we are making breaking
>> >> changes.
>> >>
>> >> IMO, the main issue here is that the release manager doesn't know the
>> PR
>> >> is
>> >> introducing breaking changes, rather than thinking that the
>> introduction
>> >> of
>> >> breaking changes is reasonable to the patch release. I noticed Jason
>> had
>> >> added the release/* label, I think he also isn't aware of the breaking
>> >> change.
>> >>
>> >> The release manager is unable to review all PRs before releasing it.
>> >> And the PR title said
>> >>
>> >> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
>> >> Deletion".
>> >>
>> >> My impression, it also should be bug fix.
>> >>
>> >> Regards,
>> >> Penghui
>> >>
>> >> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <xi...@apache.org>
>> >> wrote:
>> >>
>> >> > Hi Enrico Olivelli,
>> >> >
>> >> > Totally agree, we should be careful when cherry-picking PRs. And we
>> >> can't
>> >> > trust our own judgment too much. For an uncertain PR, we must submit
>> a
>> >> PR
>> >> > and wait for everyone to review it together.
>> >> > For example, for the PR [1] mentioned above, the measure I took was
>> to
>> >> push
>> >> > a PR to cherry-pick and move it to the next release version (2.10.5)
>> so
>> >> > that we have enough time to discuss and reach an agreement.
>> >> >
>> >> > Sincerely,
>> >> > Xiangying
>> >> > [1]
>> >> >
>> >>
>> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
>> >> >
>> >> > On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
>> >> > <yu...@streamnative.io.invalid> wrote:
>> >> >
>> >> > >  Hi Enrico Olivelli
>> >> > >
>> >> > > Thank you for helping me correct my mistake
>> >> > >
>> >> > > 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

Posted by PengHui Li <pe...@apache.org>.
> At least the RM is responsible for PRs cherry-picked he/she made. As we
take compatibility in a high priority, if it's unclear a fix (patch)
without breaking changes, the RM can ask for confirmation.

Yes, but it's not easy without going through all the comments,
review the PR changes. If we don't have an eye-catching reminder.

Thanks,
Penghui

On Tue, Feb 28, 2023 at 1:14 PM tison <wa...@gmail.com> wrote:

> For "what is breaking changes", at least the PULL_REQUEST_TEMPLATE gives
> some hints:
>
> https://github.com/apache/pulsar/blob/bbb543d8ed2b03361807c852da1a31cfb92939f3/.github/PULL_REQUEST_TEMPLATE.md?plain=1#L56-L72
>
> For the issued PR, Hang commented at
> https://github.com/apache/pulsar/pull/15914#issuecomment-1146942281 that
> it
> changes the default behavior.
>
> An alternative is that the original PR author can hint a ! in title or
> BREAKING CHANGE and commit message like conventionalcommits spec defines.
>
> Best,
> tison.
>
>
> tison <wa...@gmail.com> 于2023年2月28日周二 13:07写道:
>
> > > The release manager is unable to review all PRs before releasing it.
> >
> > At least the RM is responsible for PRs cherry-picked he/she made. As we
> > take compatibility in a high priority, if it's unclear a fix (patch)
> > without breaking changes, the RM can ask for confirmation.
> >
> > Best,
> > tison.
> >
> >
> > PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:
> >
> >> Hi enrico,
> >>
> >> +1 for your point.
> >>
> >> Do you know the details of the breaking change?
> >> I can't find any discussions under the mailing list about the breaking
> >> change.
> >>
> >> I have added the `release/important-notice ` label to the PR, and we
> >> should
> >> also discuss first, better to have a proposal if we are making breaking
> >> changes.
> >>
> >> IMO, the main issue here is that the release manager doesn't know the PR
> >> is
> >> introducing breaking changes, rather than thinking that the introduction
> >> of
> >> breaking changes is reasonable to the patch release. I noticed Jason had
> >> added the release/* label, I think he also isn't aware of the breaking
> >> change.
> >>
> >> The release manager is unable to review all PRs before releasing it.
> >> And the PR title said
> >>
> >> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
> >> Deletion".
> >>
> >> My impression, it also should be bug fix.
> >>
> >> Regards,
> >> Penghui
> >>
> >> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <xi...@apache.org>
> >> wrote:
> >>
> >> > Hi Enrico Olivelli,
> >> >
> >> > Totally agree, we should be careful when cherry-picking PRs. And we
> >> can't
> >> > trust our own judgment too much. For an uncertain PR, we must submit a
> >> PR
> >> > and wait for everyone to review it together.
> >> > For example, for the PR [1] mentioned above, the measure I took was to
> >> push
> >> > a PR to cherry-pick and move it to the next release version (2.10.5)
> so
> >> > that we have enough time to discuss and reach an agreement.
> >> >
> >> > Sincerely,
> >> > Xiangying
> >> > [1]
> >> >
> >>
> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
> >> >
> >> > On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
> >> > <yu...@streamnative.io.invalid> wrote:
> >> >
> >> > >  Hi Enrico Olivelli
> >> > >
> >> > > Thank you for helping me correct my mistake
> >> > >
> >> > > 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

Posted by tison <wa...@gmail.com>.
For "what is breaking changes", at least the PULL_REQUEST_TEMPLATE gives
some hints:
https://github.com/apache/pulsar/blob/bbb543d8ed2b03361807c852da1a31cfb92939f3/.github/PULL_REQUEST_TEMPLATE.md?plain=1#L56-L72

For the issued PR, Hang commented at
https://github.com/apache/pulsar/pull/15914#issuecomment-1146942281 that it
changes the default behavior.

An alternative is that the original PR author can hint a ! in title or
BREAKING CHANGE and commit message like conventionalcommits spec defines.

Best,
tison.


tison <wa...@gmail.com> 于2023年2月28日周二 13:07写道:

> > The release manager is unable to review all PRs before releasing it.
>
> At least the RM is responsible for PRs cherry-picked he/she made. As we
> take compatibility in a high priority, if it's unclear a fix (patch)
> without breaking changes, the RM can ask for confirmation.
>
> Best,
> tison.
>
>
> PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:
>
>> Hi enrico,
>>
>> +1 for your point.
>>
>> Do you know the details of the breaking change?
>> I can't find any discussions under the mailing list about the breaking
>> change.
>>
>> I have added the `release/important-notice ` label to the PR, and we
>> should
>> also discuss first, better to have a proposal if we are making breaking
>> changes.
>>
>> IMO, the main issue here is that the release manager doesn't know the PR
>> is
>> introducing breaking changes, rather than thinking that the introduction
>> of
>> breaking changes is reasonable to the patch release. I noticed Jason had
>> added the release/* label, I think he also isn't aware of the breaking
>> change.
>>
>> The release manager is unable to review all PRs before releasing it.
>> And the PR title said
>>
>> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
>> Deletion".
>>
>> My impression, it also should be bug fix.
>>
>> Regards,
>> Penghui
>>
>> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <xi...@apache.org>
>> wrote:
>>
>> > Hi Enrico Olivelli,
>> >
>> > Totally agree, we should be careful when cherry-picking PRs. And we
>> can't
>> > trust our own judgment too much. For an uncertain PR, we must submit a
>> PR
>> > and wait for everyone to review it together.
>> > For example, for the PR [1] mentioned above, the measure I took was to
>> push
>> > a PR to cherry-pick and move it to the next release version (2.10.5) so
>> > that we have enough time to discuss and reach an agreement.
>> >
>> > Sincerely,
>> > Xiangying
>> > [1]
>> >
>> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
>> >
>> > On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
>> > <yu...@streamnative.io.invalid> wrote:
>> >
>> > >  Hi Enrico Olivelli
>> > >
>> > > Thank you for helping me correct my mistake
>> > >
>> > > 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

Posted by tison <wa...@gmail.com>.
> The release manager is unable to review all PRs before releasing it.

At least the RM is responsible for PRs cherry-picked he/she made. As we
take compatibility in a high priority, if it's unclear a fix (patch)
without breaking changes, the RM can ask for confirmation.

Best,
tison.


PengHui Li <pe...@apache.org> 于2023年2月28日周二 12:45写道:

> Hi enrico,
>
> +1 for your point.
>
> Do you know the details of the breaking change?
> I can't find any discussions under the mailing list about the breaking
> change.
>
> I have added the `release/important-notice ` label to the PR, and we should
> also discuss first, better to have a proposal if we are making breaking
> changes.
>
> IMO, the main issue here is that the release manager doesn't know the PR is
> introducing breaking changes, rather than thinking that the introduction of
> breaking changes is reasonable to the patch release. I noticed Jason had
> added the release/* label, I think he also isn't aware of the breaking
> change.
>
> The release manager is unable to review all PRs before releasing it.
> And the PR title said
>
> "[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic
> Deletion".
>
> My impression, it also should be bug fix.
>
> Regards,
> Penghui
>
> On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <xi...@apache.org>
> wrote:
>
> > Hi Enrico Olivelli,
> >
> > Totally agree, we should be careful when cherry-picking PRs. And we can't
> > trust our own judgment too much. For an uncertain PR, we must submit a PR
> > and wait for everyone to review it together.
> > For example, for the PR [1] mentioned above, the measure I took was to
> push
> > a PR to cherry-pick and move it to the next release version (2.10.5) so
> > that we have enough time to discuss and reach an agreement.
> >
> > Sincerely,
> > Xiangying
> > [1]
> > https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
> >
> > On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
> > <yu...@streamnative.io.invalid> wrote:
> >
> > >  Hi Enrico Olivelli
> > >
> > > Thank you for helping me correct my mistake
> > >
> > > Yubiao Feng
> > >
> > > On Mon, Feb 27, 2023 at 11:27 PM Enrico Olivelli <eo...@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

Posted by PengHui Li <pe...@apache.org>.
Hi enrico,

+1 for your point.

Do you know the details of the breaking change?
I can't find any discussions under the mailing list about the breaking
change.

I have added the `release/important-notice ` label to the PR, and we should
also discuss first, better to have a proposal if we are making breaking
changes.

IMO, the main issue here is that the release manager doesn't know the PR is
introducing breaking changes, rather than thinking that the introduction of
breaking changes is reasonable to the patch release. I noticed Jason had
added the release/* label, I think he also isn't aware of the breaking
change.

The release manager is unable to review all PRs before releasing it.
And the PR title said

"[Fix][Tiered Storage] Eagerly Delete Offloaded Segments On Topic Deletion".

My impression, it also should be bug fix.

Regards,
Penghui

On Tue, Feb 28, 2023 at 10:32 AM Xiangying Meng <xi...@apache.org>
wrote:

> Hi Enrico Olivelli,
>
> Totally agree, we should be careful when cherry-picking PRs. And we can't
> trust our own judgment too much. For an uncertain PR, we must submit a PR
> and wait for everyone to review it together.
> For example, for the PR [1] mentioned above, the measure I took was to push
> a PR to cherry-pick and move it to the next release version (2.10.5) so
> that we have enough time to discuss and reach an agreement.
>
> Sincerely,
> Xiangying
> [1]
> https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022
>
> On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
> <yu...@streamnative.io.invalid> wrote:
>
> >  Hi Enrico Olivelli
> >
> > Thank you for helping me correct my mistake
> >
> > Yubiao Feng
> >
> > On Mon, Feb 27, 2023 at 11:27 PM Enrico Olivelli <eo...@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

Posted by Xiangying Meng <xi...@apache.org>.
Hi Enrico Olivelli,

Totally agree, we should be careful when cherry-picking PRs. And we can't
trust our own judgment too much. For an uncertain PR, we must submit a PR
and wait for everyone to review it together.
For example, for the PR [1] mentioned above, the measure I took was to push
a PR to cherry-pick and move it to the next release version (2.10.5) so
that we have enough time to discuss and reach an agreement.

Sincerely,
Xiangying
[1] https://github.com/apache/pulsar/pull/19640#pullrequestreview-1315805022

On Tue, Feb 28, 2023 at 9:56 AM Yubiao Feng
<yu...@streamnative.io.invalid> wrote:

>  Hi Enrico Olivelli
>
> Thank you for helping me correct my mistake
>
> Yubiao Feng
>
> On Mon, Feb 27, 2023 at 11:27 PM Enrico Olivelli <eo...@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

Posted by Yubiao Feng <yu...@streamnative.io.INVALID>.
 Hi Enrico Olivelli

Thank you for helping me correct my mistake

Yubiao Feng

On Mon, Feb 27, 2023 at 11:27 PM Enrico Olivelli <eo...@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
>