osdir.com

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

Re: PR Milestone policy


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