Re: PR Milestone policy

[Roman Leventov <le...@gmail.com>]

Looks like three committers (Jonathan, Mingming and I) voted for the policy
of not adding non bug and security PRs to milestones before merging.
One (Gian) abstained. Therefore I included this in the PR action item list
for committers: https://github.com/apache/incubator-druid/pull/7279 please
review.

On Mon, 7 Jan 2019 at 16:11, Gian Merlino <gi...@apache.org> wrote:

> My feeling is that setting a milestone on PRs before they're merged is a
> way of making their authors feel more included. I don't necessarily see a
> problem with setting milestones optimistically and then, when a release
> branch is about to be cut (based on the timed release date), we bulk-update
> anything that hasn't been merged yet to the next milestone.
>
> However, there are other ways to make authors feel more included. If we end
> up doing a more formalized proposal process then this helps too. (It should
> be easier for people to comment on proposals than on PRs, since there isn't
> a need to read code.)
>
> I guess I'm not really fussy either way on this one.
>
> On Wed, Dec 12, 2018 at 10:27 PM 邱明明 <qm...@apache.org> wrote:
>
> > I agree with Jonathan.
> > Jay Nash <ja...@gmail.com> 于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....@apache.org> 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...@apache.org>
> 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...@gmail.com>>
> > > > > 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...@apache.org>
> > 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...@gmail.com
> >>
> > > > > >> 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...@gmail.com>>
> > > > > 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...@gmail.com>>
> > > > > >> 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...@apache.org>
> > 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...@imply.io
> >>
> > > > > >> 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@gmail.com>>
> > > > > >>>>>> 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...@apache.org
> > >>
> > > > > >> 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...@imply.io
> > >>
> > > > > >> 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...@apache.org>>
> > > > > >>>> 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@apache.org>>
> > > > > >>>>>>>>> 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@druid.apache.org
> > >
> > > > > >>>>>>>>>>> For additional commands, e-mail:
> > dev-help@druid.apache.org>
> > > > > >>>>>>>>>>>>
> > > > > >>>>>>>>>>>>
> > > > > >>>>>>>>>>
> > > > > >>>>>>>>>>
> > > > > >>>>>>>>>>
> > > > > >>
> > --------------------------------------------------------------------->
> > > > > >>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
> >
> > > > > >>>>>>>>> For additional commands, e-mail:
> dev-help@druid.apache.org
> > >
> > > > > >>>>>>>>>>
> > > > > >>>>>>>>>>
> > > > > >>>>>>>>>
> > > > > >>>>>>>
> > > > > >>>>>>>
> > > > > >>>>>>>
> > > > > >>
> > --------------------------------------------------------------------->
> > > > > >>>>>> To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org>
> > > > > >>>>>> For additional commands, e-mail: dev-help@druid.apache.org>
> > > > > >>>>>>>
> > > > > >>>>>>>
> > > > > >>>>>
> > > > > >>>>
> > --------------------------------------------------------------------->
> > > > > >>>> To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org>
> > > > > >>>> For additional commands, e-mail: dev-help@druid.apache.org>
> > > > > >>>>>
> > > > > >>>>>
> > > > > >>>
> > > > > >>
> > --------------------------------------------------------------------->
> > > > > >> To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org>
> > > > > >> For additional commands, e-mail: dev-help@druid.apache.org>
> > > > > >>>
> > > > > >>>
> > > > >>
> > > > >>
> > > > >
> > --------------------------------------------------------------------->
> > > > > To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org>
> > > > > For additional commands, e-mail: dev-help@druid.apache.org>
> > > > >>
> > > > >>
> > > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
> > > For additional commands, e-mail: dev-help@druid.apache.org
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
> > For additional commands, e-mail: dev-help@druid.apache.org
> >
> >
>