nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Witt <joe.w...@gmail.com>
Subject Re: [DISCUSS] Addressing Lingering Pull Requests
Date Mon, 29 Jan 2018 17:48:48 GMT
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/de5e103994f356b1b8a572410938eef44af8cb352210e35305c04bc9@%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
View raw message