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 <wave4d...@comcast.net> wrote: > > > Sent from my iPhone > > > On Feb 27, 2023, at 9:28 PM, tison <wander4...@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 <wave4d...@comcast.net> 于2023年2月28日周二 13:21写道： > > > >> > >> > >> Sent from my iPhone > >> > >>>> On Feb 27, 2023, at 9:08 PM, tison <wander4...@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 <peng...@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 < > xiangy...@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 > >>>>> <yubiao.f...@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 < > eolive...@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 > >>>>>>> > >>>>>> > >>>>> > >>>> > >> > >> >
