[DISCUSS] Addressing Lingering Pull Requests

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[DISCUSS] Addressing Lingering Pull Requests

Mark Payne
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
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Joe Witt
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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Bryan Bende
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 <[hidden email]> 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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Aldrin Piri
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Joe Witt
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
>> 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
>>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Pierre Villard
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 <[hidden email]>:

> 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 <[hidden email]>
> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
> >> 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
> >>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Sivaprasanna Sethuraman
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 <[hidden email]>
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 <[hidden email]>:
>
> > 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 <[hidden email]>
> > 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 <[hidden email]>
> 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 <[hidden email]>
> 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 <[hidden email]>
> > >> 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
> > >>
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Aldrin Piri
Hello,

Yes, this is unrelated (or at least I believe) and is an effort within the
ASF.  There is not a lot of information available, but the associated page
is https://gitbox.apache.org/.

On Mon, Jan 29, 2018 at 9:11 PM, Sivaprasanna <[hidden email]>
wrote:

> 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 <
> [hidden email]>
> 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 <[hidden email]>:
> >
> > > 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 <[hidden email]>
> > > 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 <[hidden email]>
> > 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 <[hidden email]>
> > 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 <
> [hidden email]>
> > > >> 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
> > > >>
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Sivaprasanna Sethuraman
Thanks. I’ll take a look.

On Tue, 30 Jan 2018 at 7:49 AM, Aldrin Piri <[hidden email]> wrote:

> Hello,
>
> Yes, this is unrelated (or at least I believe) and is an effort within the
> ASF.  There is not a lot of information available, but the associated page
> is https://gitbox.apache.org/.
>
> On Mon, Jan 29, 2018 at 9:11 PM, Sivaprasanna <[hidden email]>
> wrote:
>
> > 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 <
> > [hidden email]>
> > 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 <[hidden email]>:
> > >
> > > > 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 <[hidden email]>
> > > > 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 <[hidden email]>
> > > 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 <[hidden email]>
> > > 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 <
> > [hidden email]>
> > > > >> 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
> > > > >>
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

James Wing
In reply to this post by Mark Payne
This is a great idea, Mark, thanks for proposing it.  30 days after last
review comment seems like a good, enforceable standard.

James

On Mon, Jan 29, 2018 at 8:29 AM, Mark Payne <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Mike Thomsen
It looks like there are at least 10 new processors and services in the
backlog, and quite a few modifications to existing ones.

Something I think would really help here is to expand the scope of
requirements for submitting a new processor for code review to include:

1. docker-compose file that sets up a complete development environment w/
appropriate port mappings that just work when used with NiFi running
outside of Docker on the reviewer's machine.

2. A sample flow, a really KISS example w/ GenerateFlowFile or something
that spits out a query or sample that can let the reviewer really see the
submitter's idea of what the input should look like.

Whether those are stored on a Wiki or in the Git repo doesn't really
matter. I think submitting those artifacts will really reduce the burden of
quickly going from "LGTM, JUnits seem to run" to "OK, I see it actually
running as expected."

On Tue, Jan 30, 2018 at 12:38 AM, James Wing <[hidden email]> wrote:

> This is a great idea, Mark, thanks for proposing it.  30 days after last
> review comment seems like a good, enforceable standard.
>
> James
>
> On Mon, Jan 29, 2018 at 8:29 AM, Mark Payne <[hidden email]> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Andy LoPresto-2
Mike,

I think that would be awesome for reviewers (and that is where most of my time is spent on the review side), but I also think that sets a really high bar for contribution. Many of the users who submit pull requests are first-time contributors or new to the project, and I believe the easier we can make contributing, the more we will grow and strengthen our community. Someone might have experience with a weird protocol or multithreading and be able to offer expertise there, but not have the familiarity with Docker. For experienced users contributing advanced functionality though, I think it would be excellent and definitely streamline the review process. 


Andy LoPresto
PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4  BACE 3C6E F65B 2F7D EF69

On Feb 1, 2018, at 9:31 AM, Mike Thomsen <[hidden email]> wrote:

It looks like there are at least 10 new processors and services in the
backlog, and quite a few modifications to existing ones.

Something I think would really help here is to expand the scope of
requirements for submitting a new processor for code review to include:

1. docker-compose file that sets up a complete development environment w/
appropriate port mappings that just work when used with NiFi running
outside of Docker on the reviewer's machine.

2. A sample flow, a really KISS example w/ GenerateFlowFile or something
that spits out a query or sample that can let the reviewer really see the
submitter's idea of what the input should look like.

Whether those are stored on a Wiki or in the Git repo doesn't really
matter. I think submitting those artifacts will really reduce the burden of
quickly going from "LGTM, JUnits seem to run" to "OK, I see it actually
running as expected."

On Tue, Jan 30, 2018 at 12:38 AM, James Wing <[hidden email]> wrote:

This is a great idea, Mark, thanks for proposing it.  30 days after last
review comment seems like a good, enforceable standard.

James

On Mon, Jan 29, 2018 at 8:29 AM, Mark Payne <[hidden email]> 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



signature.asc (891 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Addressing Lingering Pull Requests

Matt Burgess-2
I agree with Andy on the Docker point, I think it could be too high a
barrier for contribution in some cases. However I think we can build
out / extend a "new component" section of the PR template that has
more best practices, recommendations, suggestions.

I like the idea of the reviewer providing a sample template, perhaps
via Gist or something, but that should probably start as a
recommendation and not a requirement. If a reviewer is unfamiliar with
the new component (or its third-party libraries), they can always ask
the author if they have a test flow they can share, and hopefully the
practice will become status quo organically.

Another thing we could add is checkbox(es) around standard efforts
associated with the addition of new components/bundles. Specifically
if you add new NARs, you have to add them to the nifi-assembly POM and
their versions to the top-level POM, not to mention the bundle to the
nifi-nar-bundles POM.

Regards,
Matt


On Thu, Feb 1, 2018 at 10:20 AM, Andy LoPresto <[hidden email]> wrote:

> Mike,
>
> I think that would be awesome for reviewers (and that is where most of my
> time is spent on the review side), but I also think that sets a really high
> bar for contribution. Many of the users who submit pull requests are
> first-time contributors or new to the project, and I believe the easier we
> can make contributing, the more we will grow and strengthen our community.
> Someone might have experience with a weird protocol or multithreading and be
> able to offer expertise there, but not have the familiarity with Docker. For
> experienced users contributing advanced functionality though, I think it
> would be excellent and definitely streamline the review process.
>
>
> Andy LoPresto
> [hidden email]
> [hidden email]
> PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4  BACE 3C6E F65B 2F7D EF69
>
> On Feb 1, 2018, at 9:31 AM, Mike Thomsen <[hidden email]> wrote:
>
> It looks like there are at least 10 new processors and services in the
> backlog, and quite a few modifications to existing ones.
>
> Something I think would really help here is to expand the scope of
> requirements for submitting a new processor for code review to include:
>
> 1. docker-compose file that sets up a complete development environment w/
> appropriate port mappings that just work when used with NiFi running
> outside of Docker on the reviewer's machine.
>
> 2. A sample flow, a really KISS example w/ GenerateFlowFile or something
> that spits out a query or sample that can let the reviewer really see the
> submitter's idea of what the input should look like.
>
> Whether those are stored on a Wiki or in the Git repo doesn't really
> matter. I think submitting those artifacts will really reduce the burden of
> quickly going from "LGTM, JUnits seem to run" to "OK, I see it actually
> running as expected."
>
> On Tue, Jan 30, 2018 at 12:38 AM, James Wing <[hidden email]> wrote:
>
> This is a great idea, Mark, thanks for proposing it.  30 days after last
> review comment seems like a good, enforceable standard.
>
> James
>
> On Mon, Jan 29, 2018 at 8:29 AM, Mark Payne <[hidden email]> 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
>
>
>