osdir.com

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

Re: PR Milestone policy


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