osdir.com


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

Re: PR Milestone policy


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