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