osdir.com

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: PR Milestone policy


I agree with Jonathan.
Jay Nash <jaynasht@xxxxxxxxx> 于2018年12月13日周四 下午1:05写道:
>
> Dear all,
> I am just bystander on Druid List however I like to contribute code to Druids some day because it is very great, we use it at my company. It sounds consensus was reached that Github milestones should be used not so frequently and is proposed vote about to change this.. is this correct?
>
> Regards,
> Jay
>
> On 2018/12/12 00:39:29, Jonathan Wei <j...@xxxxxxxxxx> wrote:
> > After a PR has been reviewed and merged, I think we should tag it with the>
> > upcoming milestone to make life easier for release managers, for all PRs.>
> >
> > Regarding unresolved PRs:>
> >
> > > I advocate for not assigning milestones to any non-bug (or otherwise>
> > "critical") PRs, including "feature", non-refactoring PRs.>
> >
> > That seems like a reasonable policy to me, based on the points Roman made>
> > in the thread.>
> >
> > On Tue, Dec 11, 2018 at 1:13 AM Julian Hyde <jh...@xxxxxxxxxx> wrote:>
> >
> > > Well, see if you can get consensus around such a policy. Other Druid>
> > > folks, please speak up if you agree or disagree.>
> > >>
> > > > On Dec 8, 2018, at 8:02 AM, Roman Leventov <le...@xxxxxxxxx>>
> > > wrote:>
> > > >>
> > > > It's not exactly and not only that. I advocate for not assigning>
> > > milestones>
> > > > to any non-bug (or otherwise "critical") PRs, including "feature",>
> > > > non-refactoring PRs.>
> > > >>
> > > > On Fri, 7 Dec 2018 at 19:29, Julian Hyde <jh...@xxxxxxxxxx> wrote:>
> > > >>
> > > >> Consensus.>
> > > >>>
> > > >> We resolve debates by going into them knowing that we need to find>
> > > >> consensus. A vote is a last step to prove that consensus exists, and>
> > > >> in most cases is not necessary.>
> > > >>>
> > > >> Reading between the lines, it sounds as if you and FJ have a>
> > > >> difference of opinion about refactoring changes. Two extreme positions>
> > > >> would be (1) we don't accept changes that only refactor code, (2) and>
> > > >> I assert my right to contribute a refactoring change at any point in>
> > > >> the project lifecycle. A debate that starts with those positions is>
> > > >> never going to reach consensus. A better starting point might be "I>
> > > >> would like to make the following change because I believe it would be>
> > > >> beneficial. How could I best structure it / time it to minimize>
> > > >> impact?">
> > > >> On Fri, Dec 7, 2018 at 9:19 AM Roman Leventov <le...@xxxxxxxxx>>
> > > >> wrote:>
> > > >>>>
> > > >>> I would like like learn what is the Apache way to resolve debates. But>
> > > >> you>
> > > >>> are right, this question probably doesn't deserve that. Thanks for>
> > > >> guidance>
> > > >>> Julian.>
> > > >>>>
> > > >>> On Fri, 7 Dec 2018 at 16:43, Julian Hyde <jh...@xxxxxxxxx>>
> > > wrote:>
> > > >>>>
> > > >>>> May I suggest that a vote is not the solution. In this discussion I>
> > > see>
> > > >>>> two people beating each other over the head with policy.>
> > > >>>>>
> > > >>>> Let’s strive to operate according to the Apache way. Accept>
> > > >> contributions>
> > > >>>> on merit in a timely manner. Avoid the urge to “project manage”.>
> > > >>>>>
> > > >>>> Julian>
> > > >>>>>
> > > >>>>> On Dec 7, 2018, at 07:03, Roman Leventov <le...@xxxxxxxxx>>
> > > >> wrote:>
> > > >>>>>>
> > > >>>>> The previous consensus community decision seems to be to not use PR>
> > > >>>>> milestones for any PRs except bugs. To change this policy, probably>
> > > >> there>
> > > >>>>> should be a committer (or PPMC?) vote.>
> > > >>>>>>
> > > >>>>>> On Thu, 6 Dec 2018 at 20:49, Julian Hyde <jh...@xxxxxxxxxx> wrote:>
> > > >>>>>>>
> > > >>>>>> FJ,>
> > > >>>>>>>
> > > >>>>>> What you are proposing sounds suspiciously like project management.>
> > > >> If a>
> > > >>>>>> contributor makes a contribution, that contribution should be given>
> > > >> a>
> > > >>>> fair>
> > > >>>>>> review in a timely fashion and be committed based on its merits. You>
> > > >>>>>> overstate the time-sensitivity of contributions. I would imagine>
> > > >> that>
> > > >>>> there>
> > > >>>>>> are only a few days preceding each release where stability is a>
> > > >> major>
> > > >>>>>> concern. At any other times, contributions can go in after a review.>
> > > >>>>>>>
> > > >>>>>> Remember that in Apache, no one person or group of people>
> > > >> determines the>
> > > >>>>>> technical direction of the project, nor the timing of the releases.>
> > > >>>>>> Contributions are accepted based on merit, and release timing is>
> > > >>>> determined>
> > > >>>>>> by consensus.>
> > > >>>>>>>
> > > >>>>>> Let’s be sure not to overuse milestone policy. Milestones should be>
> > > >> for>
> > > >>>>>> guidance only.>
> > > >>>>>>>
> > > >>>>>> Julian>
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>> On Dec 6, 2018, at 10:12 AM, Fangjin Yang <fa...@xxxxxxxx>>
> > > >> wrote:>
> > > >>>>>>>>
> > > >>>>>>> Roman - one of the roles of a committer is to make decisions on>
> > > >> what is>
> > > >>>>>>> best for Druid and the Druid community. If a committer feels that>
> > > >> their>
> > > >>>>>> PR>
> > > >>>>>>> should be included in the next release, they should make an>
> > > >> argument of>
> > > >>>>>> why>
> > > >>>>>>> that is. Conversely, if folks in the community feel that a PR>
> > > >> should>
> > > >>>> not>
> > > >>>>>> be>
> > > >>>>>>> included, they should be free to voice their opinion as well.>
> > > >>>>>>>>
> > > >>>>>>> Many of the community contributions I see today are adding value>
> > > >> to the>
> > > >>>>>>> project and we should try to include them in upcoming releases. The>
> > > >>>> PRs I>
> > > >>>>>>> see adding no value are unnecessary refactoring of that serve no>
> > > >> real>
> > > >>>>>>> purpose. They don't make the code stable, easier to maintain, or>
> > > >> add>
> > > >>>> new>
> > > >>>>>>> features, and look to be submitted only to increase total>
> > > >> contribution>
> > > >>>>>> line>
> > > >>>>>>> count to Druid. I think we should aim to prevent these types of>
> > > >> PRs in>
> > > >>>>>> any>
> > > >>>>>>> release because they don't serve to benefit the community.>
> > > >>>>>>>>
> > > >>>>>>> On Tue, Dec 4, 2018 at 5:24 AM Roman Leventov <>
> > > >> leventov.ru@xxxxxxxxx>>
> > > >>>>>> wrote:>
> > > >>>>>>>>
> > > >>>>>>>> Fangjin, what you suggest will lead to just one thing - all>
> > > >> committers>
> > > >>>>>> will>
> > > >>>>>>>> always assign their PRs to the next release milestone. In>
> > > >> addition,>
> > > >>>> you>
> > > >>>>>>>> also assign PRs from non-committers to the next release>
> > > >> milestone. So>
> > > >>>>>>>> nearly 100% of new PRs will have that milestone. It will make this>
> > > >>>> whole>
> > > >>>>>>>> activity pointless, because the milestone will not tell release>
> > > >>>> managers>
> > > >>>>>>>> anything. Except maybe creating unneeded sense of rush.>
> > > >>>>>>>>>
> > > >>>>>>>> Currently in Druid, there is a good fraction of PRs that are>
> > > >> merged>
> > > >>>> very>
> > > >>>>>>>> quickly (in a matter of days and sometimes hours), but there are>
> > > >> also>
> > > >>>>>> quite>
> > > >>>>>>>> some less lucky PRs that linger for months. For contributors,>
> > > >> it's not>
> > > >>>>>> very>
> > > >>>>>>>> important that the PR is merged in 1 hour, it's more important>
> > > >> that it>
> > > >>>>>>>> appears in the next release. Therefore we need to optimize for the>
> > > >>>>>> fraction>
> > > >>>>>>>> of PRs that are merged in 1 month or less (the average time>
> > > >> between>
> > > >>>>>>>> creation of a new release branch and a final release date).>
> > > >> Reviewers>
> > > >>>>>>>> should schedule their time so that there are less PRs that are>
> > > >> merged>
> > > >>>> in>
> > > >>>>>>>> less than one day, but more PRs that are merged in less than one>
> > > >>>> month.>
> > > >>>>>>>>>
> > > >>>>>>>>> On Tue, 4 Dec 2018 at 04:28, Julian Hyde <jh...@xxxxxxxxxx>>
> > > >> wrote:>
> > > >>>>>>>>>>
> > > >>>>>>>>> I agree with you that merging PRs promptly is very important for>
> > > >>>>>> growing>
> > > >>>>>>>>> community. Or, if the PR is inadequate, promptly explain to the>
> > > >>>>>>>> contributor>
> > > >>>>>>>>> what they can do to improve it.>
> > > >>>>>>>>>>
> > > >>>>>>>>> Assigning target milestones to bugs and issues that don’t yet>
> > > >> have>
> > > >>>> PRs>
> > > >>>>>>>> can>
> > > >>>>>>>>> be problematic. The person assigning the milestone has stepped>
> > > >> into>
> > > >>>> the>
> > > >>>>>>>>> role of project manager, unless they are committing to fix the>
> > > >> issue>
> > > >>>>>>>>> personally. And even then, they are implicitly saying “hold the>
> > > >>>> release>
> > > >>>>>>>>> while I work on this code”, which should really be the>
> > > >> responsibility>
> > > >>>>>> of>
> > > >>>>>>>>> the release manager alone.>
> > > >>>>>>>>>>
> > > >>>>>>>>> Julian>
> > > >>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>> On Dec 3, 2018, at 1:57 PM, Fangjin Yang <fa...@xxxxxxxx>>
> > > >> wrote:>
> > > >>>>>>>>>>>
> > > >>>>>>>>>> Committers. In general I think we should try to be more>
> > > >> inclusive of>
> > > >>>>>>>> the>
> > > >>>>>>>>>> community and people that are interested in contributing to>
> > > >> Druid>
> > > >>>> and>
> > > >>>>>>>> try>
> > > >>>>>>>>>> to get their PRs in as much as possible. This helps to grow the>
> > > >>>>>>>>> community.>
> > > >>>>>>>>>> To me, this means assigning milestones to contributions, not>
> > > >> being>
> > > >>>>>>>> overly>
> > > >>>>>>>>>> picky on code (if it has no real impact on>
> > > >>>> functionality/performance).>
> > > >>>>>>>> If>
> > > >>>>>>>>>> we need to push PRs back to a later release because they are>
> > > >>>>>>>> complicated>
> > > >>>>>>>>>> and require more review, we can always do that.>
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> On Tue, Nov 27, 2018 at 4:45 PM Julian Hyde <jh...@xxxxxxxxxx>>
> > > >>>> wrote:>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>> Fangjin,>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>> You wrote>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>> we should try to assign milestones to PRs we want>
> > > >>>>>>>>>>>> to get in>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>> Can you please define “we”? Do you mean committers, PMC>
> > > >> members,>
> > > >>>>>>>> release>
> > > >>>>>>>>>>> managers, everyone?>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>> Julian>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>> On Nov 26, 2018, at 8:43 AM, Roman Leventov <>
> > > >> leventov@xxxxxxxxxx>>
> > > >>>>>>>>> wrote:>
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> About a year ago, Gian wrote (>
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>
> > > >>>>>
> > > >>>
> > > https://groups.google.com/forum/#!msg/druid-development/QPZUIzLtZ2I/eZyw8BoVCgAJ>
> > > >>>>>>>>>>>> ):>
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> "For milestones, I think it would work to use them only for>
> > > >>>>>>>> PRs/issues>
> > > >>>>>>>>>>> that>
> > > >>>>>>>>>>>> are truly release blockers -- should be limited to critical>
> > > >> bugs>
> > > >>>>>>>>>>> discovered>
> > > >>>>>>>>>>>> after a release branch is cut. We could make release notes>
> > > >> the way>
> > > >>>>>>>> you>
> > > >>>>>>>>>>>> suggest, by walking the commit history.">
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> Today, Fangjin wrote (>
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>
> > > >>>>>
> > > >>>
> > > https://github.com/apache/incubator-druid/pull/6656#issuecomment-441698159>
> > > >>>>>>>>>>> ):>
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> "I think where possible we should try to assign milestones to>
> > > >> PRs>
> > > >>>> we>
> > > >>>>>>>>> want>
> > > >>>>>>>>>>>> to get in and aim to have the PR reviewed and merged before>
> > > >> then.>
> > > >>>> If>
> > > >>>>>>>>> the>
> > > >>>>>>>>>>> PR>
> > > >>>>>>>>>>>> needs to be pushed back to a future release we can always do>
> > > >>>> that.">
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> Personally I don't agree with the second take and>
> > > >> differentiating>
> > > >>>>>>>>> non-bug>
> > > >>>>>>>>>>>> fixing PRs by their "importance". I think the proportion of>
> > > >> PRs>
> > > >>>> that>
> > > >>>>>>>>> are>
> > > >>>>>>>>>>>> assigned the next milestone by committer will depend on>
> > > >>>>>>>> self-confidence>
> > > >>>>>>>>>>> of>
> > > >>>>>>>>>>>> the committer and politics, not the objective importance of>
> > > >> the>
> > > >>>> PRs.>
> > > >>>>>>>> It>
> > > >>>>>>>>>>>> will also make possible for some minor PRs to be sidetracked>
> > > >> for>
> > > >>>>>>>>>>> extremely>
> > > >>>>>>>>>>>> long time if not forever, because there always other more>
> > > >>>> important>
> > > >>>>>>>> PRs>
> > > >>>>>>>>>>> to>
> > > >>>>>>>>>>>> work on. While true in the short and mid run, this is very>
> > > >>>>>>>> frustrating>
> > > >>>>>>>>>>> for>
> > > >>>>>>>>>>>> the authors and could turn them away from contributing into>
> > > >> Druid,>
> > > >>>>>>>> that>
> > > >>>>>>>>>>> is>
> > > >>>>>>>>>>>> bad in the long run. Actually this thing happens already>
> > > >> sometimes>
> > > >>>>>>>> and>
> > > >>>>>>>>> we>
> > > >>>>>>>>>>>> should think how to address that, but differentiating PRs>
> > > >> could>
> > > >>>>>>>>>>>> only exacerbate this effect.>
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> Instead, I think the importance of PR should grow with the>
> > > >> time>
> > > >>>>>>>> passed>
> > > >>>>>>>>>>>> since the author addressed all comments (or just created the>
> > > >> PR)>
> > > >>>> and>
> > > >>>>>>>>> the>
> > > >>>>>>>>>>> PR>
> > > >>>>>>>>>>>> passed automated checks. I. e. a freshly created PR may be not>
> > > >>>> super>
> > > >>>>>>>>>>>> important, but if it passes all checks and is open for two>
> > > >> months>
> > > >>>>>>>>> without>
> > > >>>>>>>>>>>> reviews, the PR becomes more important to review. This should>
> > > >> help>
> > > >>>>>> to>
> > > >>>>>>>>>>>> reduce the variance in PR's time-to-merge and improve the>
> > > >> average>
> > > >>>>>>>>>>>> contributor experience. In the long run I think it's healthier>
> > > >>>> than>
> > > >>>>>>>>>>>> squeezing one extra feature into the very next release.>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>>
> > > >>>> --------------------------------------------------------------------->
> > > >>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxx>
> > > >>>>>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxx>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>>
> > > >> --------------------------------------------------------------------->
> > > >>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxx>
> > > >>>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxx>
> > > >>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>>
> > > >> --------------------------------------------------------------------->
> > > >>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxx>
> > > >>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxx>
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>
> > > >>>> --------------------------------------------------------------------->
> > > >>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxx>
> > > >>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxx>
> > > >>>>>
> > > >>>>>
> > > >>>
> > > >> --------------------------------------------------------------------->
> > > >> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxx>
> > > >> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxx>
> > > >>>
> > > >>>
> > >>
> > >>
> > > --------------------------------------------------------------------->
> > > To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxx>
> > > For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxx>
> > >>
> > >>
> >
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxx
> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxx
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxx
For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxx