nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sivaprasanna <sivaprasanna...@gmail.com>
Subject Re: [DISCUSS] Addressing Lingering Pull Requests
Date Tue, 30 Jan 2018 02:11:40 GMT
Apologies. I’m very new to contributing so I’m not aware of gitbox. One
question this gitbox that is being discussed here is in no way related to
this one http://www.gitboxapp.com/. Correct?

On Tue, 30 Jan 2018 at 1:13 AM, Pierre Villard <pierre.villard.fr@gmail.com>
wrote:

> Agree with everything being said here. We clearly need to better manage the
> number of opened PRs and also remind the community that contributing code
> is great but that helping in the review process will create a virtuous
> circle and benefit the community.
>
> Pierre
>
> 2018-01-29 18:48 GMT+01:00 Joe Witt <joe.witt@gmail.com>:
>
> > maybe kick that gitbox thread to a vote since it is decent change to
> > the workflow.
> >
> > Thanks
> >
> > On Mon, Jan 29, 2018 at 12:45 PM, Aldrin Piri <aldrinpiri@gmail.com>
> > wrote:
> > > Gitbox was favorably received when we discussed it prior:
> > > https://lists.apache.org/thread.html/de5e103994f356b1b8a572410938ee
> > f44af8cb352210e35305c04bc9@%3Cdev.nifi.apache.org%3E
> > >
> > > I would be in favor of moving ahead with it and would be happy to get
> > > things moving if it still seems agreeable.
> > >
> > > --aldrin
> > >
> > > On Mon, Jan 29, 2018 at 11:46 AM, Bryan Bende <bbende@gmail.com>
> wrote:
> > >
> > >> I definitely agree with all of these points.
> > >>
> > >> With our current setup, the only way a committer can close a PR is by
> > >> issuing a commit with the magic "This closes ..." clause.  The
> > >> submitter of the PR is the only one who can actually close it in
> > >> GitHub.
> > >>
> > >> I don't want to hijack the discussion with a different topic, but it
> > >> might be worth considering switching to the ASF's GitBox integration
> > >> [1], which I believe lets us use Github as a real repository, rather
> > >> than just a mirror.
> > >>
> > >> It seems like it would make it easier to manage the PRs in the event
> > >> that we did implement a policy like Mark and Joe described.
> > >>
> > >> [1] https://gitbox.apache.org/repos/asf
> > >>
> > >> On Mon, Jan 29, 2018 at 11:34 AM, Joe Witt <joe.witt@gmail.com>
> wrote:
> > >> > Mark
> > >> >
> > >> > Thanks for brining this up.  I do agree.
> > >> >
> > >> > We need to probably provide more description on the contributor
> guide
> > >> > or elsewhere of which aspects makes PRs easier to commit:
> > >> >  - They have unit tests which cover core capabilities but if they're
> > >> > cloud service dependent or highly network/disk oriented they have
> > >> > integration tests instead of unit tests for the high risk or
> > >> > environmentally sensitive bits.
> > >> >  - They have *thoroughly* reviewed and covered License and Notice
> > >> > updates and are done consistently with the L&N of the rest of
the
> > >> > project.
> > >> >  - They pass all checks on Travis-CI
> > >> >  - If they required manual integration tests that detailed
> > >> > instructions/explanations of external system setup and configuration
> > >> > and test processes is provided.
> > >> >
> > >> > And maybe some explanation of which items are very difficult to get
> > >> > good reviewer help on:
> > >> > - Things which integrate with external systems that are not easily
> > >> > replicated for testing.  Consider whiz-bang database StoreIt.  If
we
> > >> > dont have others aware of or famiilar with the StoreIt system it is
> > >> > really tough to find a good reviewer and timely response.
> > >> >
> > >> > We also need to revisit this as we progress an extension registry
> > >> mechanism.
> > >> >
> > >> > Thanks
> > >> >
> > >> > On Mon, Jan 29, 2018 at 11:29 AM, Mark Payne <markap14@hotmail.com>
> > >> wrote:
> > >> >> All,
> > >> >>
> > >> >> We do from time to time go through the backlog of PR's that need
to
> > be
> > >> reviewed and
> > >> >> start a "cleansing" process, closing out any old PR's that appear
> to
> > >> have stalled out.
> > >> >> When we do this, though, we typically will start sending out
> e-mails
> > >> asking if there are
> > >> >> any stalled PR's that we shouldn't close and start trying to
> decipher
> > >> which ones are okay
> > >> >> to close out and which ones are not. This puts quite an onus on
the
> > >> committer who is
> > >> >> trying to clean this up. It also can result in having a large
> number
> > of
> > >> outstanding Pull Requests,
> > >> >> which I believe makes the community look bad because it gives
the
> > >> appearance that we are
> > >> >> not doing a good job of being responsive to Pull Requests that
are
> > >> submitted.
> > >> >>
> > >> >> I would like to propose that we set a new "standard" that is:
if we
> > >> have any Pull Request
> > >> >> that has been stalled (and by "stalled" I mean a committer has
> > reviewed
> > >> the PR and did
> > >> >> not merge but asked for clarifications or modifications and the
> > >> contributor has not pushed
> > >> >> any new commit or responded to the comments) for at least 30 days,
> > that
> > >> we go ahead
> > >> >> and close the Pull Request (after commenting on the PR that it
is
> > being
> > >> closed due to a lack
> > >> >> of activity and that the contributor is more than welcome to open
a
> > new
> > >> PR if necessary).
> > >> >>
> > >> >> I feel like this gives contributors enough time to address concerns
> > and
> > >> it is simple enough
> > >> >> to create a new Pull Request if the need arises. Alternatively,
if
> > the
> > >> contributor realizes that
> > >> >> they need more time, they can simply comment on the PR that they
> are
> > >> still interested in
> > >> >> working on it but just need more time, and the simple act of
> > commenting
> > >> will mean that the
> > >> >> PR is no longer stalled, as defined above.
> > >> >>
> > >> >> Any thoughts on such a proposal? Any better alternatives that
> people
> > >> have in mind?
> > >> >>
> > >> >> Thanks
> > >> >> -Mark
> > >>
> >
>

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