osdir.com

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

Re: PR Milestone policy


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 <leventov.ru@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 <jhyde@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 <leventov.ru@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 <jhyde.apache@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 <leventov.ru@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 <jhyde@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 <fangjin@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 <jhyde@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 <fangjin@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 <jhyde@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