codehaus


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

Re: PR Milestone policy


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