nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pierre Villard <pierre.villard...@gmail.com>
Subject Re: [DISCUSS] Stale PRs
Date Tue, 18 Sep 2018 13:39:14 GMT
Otto, I think using the embedded option offered by Github is better as I
don't think submitting requests to Apache Infra for this is great.

What would be the best way forward on this subject?
Andy, do you still have concerns and/or comments based on the feedback?
Should it go through a formal VOTE thread?

Happy to take care of it if there is a consensus.

Thanks,
Pierre


Le dim. 16 sept. 2018 à 15:38, Otto Fowler <ottobackwards@gmail.com> a
écrit :

> In Metron we just put something like this in place, but not using a bot.
> We limit it to PR’s where the contributor has gone inactive.
> https://cwiki.apache.org/confluence/display/METRON/Development+Guidelines
>
> 2.6.1 Inactive Pull Requests
>
> Contributions can often take a significant amount of time to complete the
> code review process. This process requires active participation from the
> contributor. If the contributor is unable to actively participate, the pull
> request is unlikely to successfully complete this process.
>
> Pull Requests that have failed to receive active participation from the
> contributor for an extended period of time risk being abandoned. Any
> committer can submit a request for Apache Infra to close a pull request
> that has been abandoned according to the following guidelines.
>
>    - A pull request is 'inactive' if no comments or updates have been made
>    by the contributor in the previous 4 weeks.
>
>
>    - For any 'inactive' pull request, a committer can request from the
>    contributor justification for keeping the pull request open.
>
>
>    - The committer's request should be made as a public comment on the pull
>    request. The committer should refer the contributor to these development
>    guidelines for inactive pull requests.
>
>
>    - If the contributor publically responds to the request, the pull
>    request is no longer consider 'inactive'.
>
>
>    - If the contributor does not respond to the request within 2 weeks, the
>    pull request is considered 'abandoned'.
>
>
>    - A committer can cast a -1 vote on any 'abandoned' pull request using
>    these development guidelines as justification.
>
>
>    - A committer can submit a request to Apache Infra to close the
>    'abandoned' pull request based on this -1 vote.
>
>
>
> On September 15, 2018 at 22:22:17, Mike Thomsen (mikerthomsen@gmail.com)
> wrote:
>
> I am in favor of these changes. One thing we should consider is looking for
> old PRs that are close enough to being done that we could rebase them,
> clean them up a little and add them to master. Just a thought.
>
> Thanks,
>
> Mike
>
> On Sat, Sep 15, 2018 at 1:34 PM Mark Payne <markap14@hotmail.com> wrote:
>
> > I'm 100% on-board here. I brought up this same topic a couple of months
> > ago,
> > but the thread kind of digressed (as these things tend to do on large
> > mailing lists).
> > I am in favor of a 30 day period with a reminder that gives the
> > contributor an extra
> > week before closing the PR. If the contributor is simply busy and not
> able
> > to finish up
> > for a while, a simple comment to that effect would allow the PR to stay
> > open. At least
> > this way we know whether a PR is in-progress or just lingering and will
> > never get
> > any progress.
> >
> > While there are times that the committers are at fault, I think that's a
> > separate discussion
> > that we can have. Both sides of the equation have to be addressed, and
> > that should not
> > prevent us from closing out stale PR's.
> >
> > That being said, I think closing out stale PR's will actually improve the
> > committers' review
> > rate. I sometimes start looking through the list of PR's to review and
> > then get overwhelmed
> > because there are so many of them right now, and almost all of them have
> a
> > comment of
> > some sort on them. Often I have no idea if the PR is still being worked
> on
> > or not. If we reduce
> > the number of open PR's down to only those that are still being worked,
> > it's a lot less overwhelming
> > for the committers to look through and see what needs to be reviewed.
> >
> > It's also worth nothing that this is not just something that Beam does. I
> > know Kubernetes has a similar
> > mechanism in place, and I'm guessing that this is a pretty common
> practice
> > in general. And one that
> > I think will definitely help out both committers and contributors and
> make
> > the project more approachable
> > from those who are interested in getting involved.
> >
> > Thanks
> > -Mark
> >
> >
> > > On Sep 15, 2018, at 12:08 PM, Jeremy Dyer <jdye64@gmail.com> wrote:
> > >
> > > Andy - That’s a good point. What I had in my mind was sort of like this
> > ....
> > >
> > > - After 30 days alert is sent to author if no activity/comment not just
> > commits.They would still have something like a week
> > > - If code is complicated they don’t have to finish it just simply
> > comment on PR to stop it from being closed
> > > - I like this because when they comment they can say things like.
> “Sorry
> > this is taking longer because of problem XYZ I’m having” others in the
> > community can see this and offer input so it helps on collaboration
> > > - This also helps people watching the PRs and interested in using them
> > have a much more clear and accurate understanding of where the PR
> actually
> > stands progress wise so they can more accurately determine when it will
> be
> > available to them
> > > - To your last point, which is a good one, they would simply comment
> > with a gentle reminder that they would like for someone to review.
> > >
> > > Thoughts? I’m sure I’m missing something there but that’s sort of how
I
> > imagine it working
> > >
> > > - Jeremy Dyer
> > >
> > > Thanks - Jeremy Dyer
> > >
> > > ________________________________
> > > From: Andy LoPresto <alopresto.apache@gmail.com>
> > > Sent: Saturday, September 15, 2018 11:57 AM
> > > To: dev@nifi.apache.org
> > > Subject: Re: [DISCUSS] Stale PRs
> > >
> > > Jeremy,
> > >
> > > What about in the scenario where the submitter does everything and we
> > (the committers) are slow? I’m not saying we shouldn’t try to fix all the
> > problems, just that I see a lot more of the latter happening.
> > >
> > > Andy LoPresto
> > > alopresto@apache.org
> > > alopresto.apache@gmail.com
> > > PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4 BACE 3C6E F65B 2F7D EF69
> > >
> > >> On Sep 15, 2018, at 08:51, Pierre Villard <
> pierre.villard.fr@gmail.com>
>
> > wrote:
> > >>
> > >> Andy,
> > >>
> > >> Totally get your points. I imagine that introducing this approach
> would
> > >> help keeping dynamic exchanges on pull requests.
> > >>
> > >> In case a PR needs complex/time consuming work (or in case the author
> is
> > >> just not in a position to process comments), I think we could have two
> > >> approaches:
> > >> - the PR is considered stale after 60 days but is actually closed one
> > week
> > >> later. I think it leaves time for someone (ideally the author) to
> > comment
> > >> and give an update so that the PR is not considered stale anymore, no?
> > >> - for important PRs, it's possible to "remove" this mechanism using
> > >> specific labels but I guess we would have to ask ASF infra if we want
> to
> > >> have rights to add labels on PRs (?)
> > >>
> > >> Pierre
> > >>
> > >>
> > >>
> > >>
> > >> Le sam. 15 sept. 2018 à 17:44, Andy LoPresto <
> > alopresto.apache@gmail.com> a
> > >> écrit :
> > >>
> > >>> Pierre,
> > >>>
> > >>> I’m going to delay my response on that proposal while I ask for (aka
> > >>> should gather on my own) some information. Is that really our
> problem?
> > By
> > >>> that, I mean are stale PRs where we are getting bogged down? I am
> sure
> > >>> there are some old ones that should be closed out. My larger concern
> is
> > >>> that even new PRs don’t get reviewed immediately for a number of
> > reasons.
> > >>>
> > >>> 1. Balance of committers to submissions. As the project continues to
> > grow,
> > >>> we have far more people providing code than can review it.
> > >>> 2. Quality of PR. Not that the code is necessarily bad, but the PR
> > doesn’t
> > >>> clearly explain the problem and how they are solving it, provide test
> > >>> cases, provide templates or a Docker container if interacting with
an
> > >>> external service, etc. All of these things add up to make the cost
of
> > >>> reviewing higher.
> > >>> 3. What PRs cover. Previously, there was still a lot of low-hanging
> > fruit,
> > >>> and less complexity. While the project is still fairly cleanly
> > organized,
> > >>> many PRs now are less “add this simple functionality” and more
> “improve
> > >>> this complicated feature.”
> > >>>
> > >>> I guess I would not have a problem with your proposal, but I do
> wonder
> > if
> > >>> there are more effective ways to reduce the backlog by identifying
> > other
> > >>> areas of improvement.
> > >>>
> > >>> Andy LoPresto
> > >>> alopresto@apache.org
> > >>> alopresto.apache@gmail.com
> > >>> PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4 BACE 3C6E F65B 2F7D EF69
> > >>>
> > >>>> On Sep 15, 2018, at 08:33, Pierre Villard <
> > pierre.villard.fr@gmail.com>
> > >>> wrote:
> > >>>>
> > >>>> Hi,
> > >>>>
> > >>>> The number of open PRs is still growing and it could make think
> people
> > >>> that
> > >>>> the project is not healthy/active (even though a quick look at
the
> > last
> > >>>> commit date or the commits rate is a clear indication that the
> > project is
> > >>>> healthy).
> > >>>>
> > >>>> In order to encourage people to review code and keep active
> > discussions
> > >>> on
> > >>>> the PRs, I suggest to find a way to keep this number as small as
> > >>> possible.
> > >>>> To do so, I'd like to ask the wider community if the approach taken
> > by a
> > >>>> project like Apache Beam would be a good idea:
> > >>>>
> > >>>> "A pull request becomes stale after its author fails to respond
to
> > >>>> actionable comments for 60 days. Author of a closed pull request
is
> > >>> welcome
> > >>>> to reopen the same pull request again in the future."
> > >>>>
> > >>>> This approach is managed by a file [1] in the .github directory
of
> the
> > >>>> repository.
> > >>>>
> > >>>> What do you think about this approach?
> > >>>>
> > >>>> [1] https://github.com/apache/beam/blob/master/.github/stale.yml
> > >>>>
> > >>>> Pierre
> > >>>
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message