osdir.com

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

Re: PR Milestone policy


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