Re: [DISCUSS] Stalled PRs

[Nick Couchman <vn...@apache.org>]

On Sat, Sep 30, 2017 at 3:38 PM, Nick Couchman <vn...@apache.org> wrote:
>
>
> I did track down e-mail address (via JIRA) for the two folks with the
> aforementioned PRs and e-mailed them directly to see if they could finish
> up the PRs.  I have not heard anything back from either of them.  Will give
> them a little longer to respond, but I think these two PRs may need to be
> co-opted and closed out.  Hopefully not - I really would like to see more
> people actively contributing to the project.
>
> -Nick
>
No response from either author or either of those pull requests.

-Nick

Re: [DISCUSS] Stalled PRs

[Mike Jumper <mi...@guac-dev.org>]

On Sat, Oct 21, 2017 at 7:25 PM, Nick Couchman <vn...@apache.org> wrote:
>>
>> My blunt opinion would be that we should never take over changes. The
>> point of an open source community like Apache Guacamole is
>> collaboration. There's a big difference between a contribution from a
>> contributor who is working together with the existing development
>> community for the benefit of all, vs. abandoned code in a basket on
>> the project's doorstep. The latter is essentially a contribution of
>> technical debt, and ultimately harmful if accepted in that form.
>>
>> We should definitely give all contributors the benefit of the doubt
>> here, and there's no reason to assume that they intended to abandon
>> the changes, but closing stalled PR's is not a bad thing. Just as with
>> a JIRA issue where the reporter has ceased responding, closing things
>> just acknowledges that the issue is not moving forward, and allows the
>> community to refocus on other matters. If the contributor comes back,
>> then things can be reopened. If not, then there really is no
>> contribution, and we should just move forward as we normally would.
>>
>> - Mike
>>
>
> Mike,
> I'm sorry.  I must be driving you nuts at this point, and I'm not trying to
> do that.  Really :-).
>

No worries - I'm still sane (I think), and if I'm going nuts I
definitely don't feel that you're driving me there. :)

> I'm not entirely sure I understand what you mean by technical debt.  What
> I'm trying to do, though, is to figure out a way to recognize people's
> contributions even when they do abandon those changes.  My suggestion of
> forking an (apparently) abandoned pull request, completing it, and then
> closing out both the new and the old request is specifically so that
> people's contributions of code are recognized and so that I'm not blatantly
> ripping off work that someone else has done with commits under my name.
>

I was considering the following as given:

1) A contributor has contributed changes which did not pass review.
2) More work is needed for those changes to be acceptable.
3) If we wish to resume that work based on the contribution, then we
must do the work that the contributor is refusing to do.

That last bit, doing the work on behalf of the contributor, is the
technical debt I'm referring to. By accepting the change in part, we
are accepting the task of bringing the change up to spec in addition
to the original task of simply solving the reported issue.

Whether this is true for the particular changes in question probably
depends on the changes themselves. If the change is wonderful except
for some nitpicking, then maybe it's worth considering building upon.
My main concern comes from the following contrived scenario:

1) Some company uses an open source product but encounters an issue
2) Company fixes issue narrowly for their use case
3) Company does not want to maintain their change
4) Company authorizes individual to contribute their change so that
it's maintained by upstream instead
5) Upstream rejects change because it's too narrow or does not comply
with guidelines
6) Company/individual refuses to collaborate with upstream, the whole
point of their contribution being to cease work in the first place

I'm not saying this is what's happening here, but the point of a
contribution is to benefit the community. If the net result of a
contribution is negative, then it's not a contribution. In my view,
the contribution of source code without the corresponding willingness
to work with the community is harmful.

> So, if for no other reason than to help me understand, let's approach this
> question a different way.  Let's take GUACAMOLE-243 and focus on that one,
> specifically.  There's a bug in Guacamole, in the guacamole-auth-ldap
> extension, in the way that it handles LDAP referrals.  This bug needs to be
> fixed - it's a particularly frustrating bug for people using Active
> Directory, and, while we don't get the question every day, there have been
> several instances of people on the mailing list wondering why they can't
> query their AD servers for users, and then someone responds pointing them
> to the global catalog (or non-global-catalog - I can never keep it
> straight).  So, clearly a bug, and clearly needs to be fixed.  Adam
> contributed some code that takes care of the issue.  He got the code almost
> all the way completed, minus a couple of minor tweaks that need to happen
> in debug logging, but hasn't responded to the pull request thread in months.
>
> ...
>
> My suggestion for forking the pull request and completing it was not aimed
> at "taking over" someone else's work, but was aimed at striking a balance
> between recognizing that someone had put some time and effort into solving
> a problem and had written some code that works and is good code, and
> recognizing that, for one or more of many reasons, that person has
> apparently decided not to see that contribution through to completion.
>

I'm not sure. I can definitely see your point, and I'm certainly not
suggesting that code be stolen. If we do decide to build off a stalled
PR, naturally that would require building off those commits as they
stand. We can't squash the commits and reset authorship; it's not an
option.

Perhaps this doesn't require as much debate as we've been giving it.
We're all committing as individuals. If a particular abandoned
contribution has enough value that you wish to adopt it, you can
definitely do so. The provenance/copyright of that contribution needs
to be clear (the original commits must remain), but your changes won't
be rejected out of hand solely because they are built upon a
contribution which was abandoned.

My personal view is that doing so is rarely a good idea, and can be
potentially harmful to the community, but as long as careful
consideration is given before adopting such contributions I suppose
I'm OK with it.

- Mike

Re: [DISCUSS] Stalled PRs

[Nick Couchman <vn...@apache.org>]

>
> My blunt opinion would be that we should never take over changes. The
> point of an open source community like Apache Guacamole is
> collaboration. There's a big difference between a contribution from a
> contributor who is working together with the existing development
> community for the benefit of all, vs. abandoned code in a basket on
> the project's doorstep. The latter is essentially a contribution of
> technical debt, and ultimately harmful if accepted in that form.
>
> We should definitely give all contributors the benefit of the doubt
> here, and there's no reason to assume that they intended to abandon
> the changes, but closing stalled PR's is not a bad thing. Just as with
> a JIRA issue where the reporter has ceased responding, closing things
> just acknowledges that the issue is not moving forward, and allows the
> community to refocus on other matters. If the contributor comes back,
> then things can be reopened. If not, then there really is no
> contribution, and we should just move forward as we normally would.
>
> - Mike
>

Mike,
I'm sorry.  I must be driving you nuts at this point, and I'm not trying to
do that.  Really :-).

I'm not entirely sure I understand what you mean by technical debt.  What
I'm trying to do, though, is to figure out a way to recognize people's
contributions even when they do abandon those changes.  My suggestion of
forking an (apparently) abandoned pull request, completing it, and then
closing out both the new and the old request is specifically so that
people's contributions of code are recognized and so that I'm not blatantly
ripping off work that someone else has done with commits under my name.

So, if for no other reason than to help me understand, let's approach this
question a different way.  Let's take GUACAMOLE-243 and focus on that one,
specifically.  There's a bug in Guacamole, in the guacamole-auth-ldap
extension, in the way that it handles LDAP referrals.  This bug needs to be
fixed - it's a particularly frustrating bug for people using Active
Directory, and, while we don't get the question every day, there have been
several instances of people on the mailing list wondering why they can't
query their AD servers for users, and then someone responds pointing them
to the global catalog (or non-global-catalog - I can never keep it
straight).  So, clearly a bug, and clearly needs to be fixed.  Adam
contributed some code that takes care of the issue.  He got the code almost
all the way completed, minus a couple of minor tweaks that need to happen
in debug logging, but hasn't responded to the pull request thread in months.

As I see it, in this case, your statement, "...there really is no
contribution, and we should just move forward as we normally would" could
mean the following things:
1) Close the PR, leaving the bug unfixed.
2) Close the PR, and completely ignore the previous commits, but fix the
code, probably using code that the person has already written, but without
giving them any credit for their work.
3) Close the PR, ignore the previous commits, and try to find a way that's
"different enough" so as to avoid the appearance of plagiarizing someone
else's work.

Or do you mean something else that I'm missing here?  To me, the first
option isn't practical, because the bug needs to be fixed.  The second
option seems like the kind of thing that is going to discourage
participation in the development process, because it gives off the vibe
that the Guacamole project is willing to plagiarize work - passing code
that others have written off as our own.  The third option seems
impractical, because, first, the way they implemented it might actually be
the best method, and, second, after you've reviewed someone's code, it's
difficult (and silly) to try to pretend like you have no knowledge of that
code, or to try to code it differently just because you do have knowledge
of it.

My suggestion for forking the pull request and completing it was not aimed
at "taking over" someone else's work, but was aimed at striking a balance
between recognizing that someone had put some time and effort into solving
a problem and had written some code that works and is good code, and
recognizing that, for one or more of many reasons, that person has
apparently decided not to see that contribution through to completion.

I'm also not trying to "codify" anything - I'm not saying we should come up
with a policy, or that we should assume that people aren't going to
complete things they begin to contribute.  I want to give people the
benefit of the doubt and I want to encourage more contributions to the
project, particularly by committed, engaged, active developers.

So, is there a way you want to handle these particular stalled PRs?  Again,
I'm not saying across the board, but, taking GUACAMOLE-243, which
represents a bug that needs to be fixed, are you suggestion we just close
the PR, drop all of the commits, and start fixing it, again, from scratch?
Similar question for the SSH key size issue.  That's more of a feature than
a bug (slightly), but the changes there are so straight-forward that it
isn't like we can just pretend we haven't been through the process of
changing that once, already.  Are we just going to close that PR and
introduce a brand new PR with those modifications, as if the first PR never
existed?

Again, I'm sorry - I'm trying to understand what direction to go, here, in
order to further the development of Guacamole as a community project - one
that recognizes contributions, that has a growing community, and that is
improving in stability, security and functionality.  I'm definitely not
trying to force my direction or viewpoint here - I'm asking for guidance on
how to approach these issues in a way that doesn't ignore past
contributions, but that recognizes that these issues need to be fixed.

-Nick

Re: [DISCUSS] Stalled PRs

[Mike Jumper <mi...@guac-dev.org>]

On Tue, Oct 17, 2017 at 5:43 AM, Nick Couchman <vn...@apache.org> wrote:
>>
>> Alright, then. Guess there's nothing to do now but close the pull requests.
>> If it's possible to use ASF's deployment of Git Box (I don't know what's
>> required here), then we can do this ourselves. Otherwise, might need to
>> open a ticket with infra.
>>
>> - Mike
>>
> I'll look into Gitbox and see what I can figure out there.
>
> The other option is to fork their changes (so as to maintain their
> contributions), finish things up, and then use the magic "this closes #XXX"
> that you did on that other PR.  Is there any reason not to go that route?
>

My blunt opinion would be that we should never take over changes. The
point of an open source community like Apache Guacamole is
collaboration. There's a big difference between a contribution from a
contributor who is working together with the existing development
community for the benefit of all, vs. abandoned code in a basket on
the project's doorstep. The latter is essentially a contribution of
technical debt, and ultimately harmful if accepted in that form.

We should definitely give all contributors the benefit of the doubt
here, and there's no reason to assume that they intended to abandon
the changes, but closing stalled PR's is not a bad thing. Just as with
a JIRA issue where the reporter has ceased responding, closing things
just acknowledges that the issue is not moving forward, and allows the
community to refocus on other matters. If the contributor comes back,
then things can be reopened. If not, then there really is no
contribution, and we should just move forward as we normally would.

- Mike

Re: [DISCUSS] Stalled PRs

[Nick Couchman <vn...@apache.org>]

>
> Alright, then. Guess there's nothing to do now but close the pull requests.
> If it's possible to use ASF's deployment of Git Box (I don't know what's
> required here), then we can do this ourselves. Otherwise, might need to
> open a ticket with infra.
>
> - Mike
>
I'll look into Gitbox and see what I can figure out there.

The other option is to fork their changes (so as to maintain their
contributions), finish things up, and then use the magic "this closes #XXX"
that you did on that other PR.  Is there any reason not to go that route?

-Nick

Re: [DISCUSS] Stalled PRs

[Nick Couchman <vn...@apache.org>]

>
>
> Alright, then. Guess there's nothing to do now but close the pull requests.
> If it's possible to use ASF's deployment of Git Box (I don't know what's
> required here), then we can do this ourselves. Otherwise, might need to
> open a ticket with infra.
>
> - Mike
>

On the Gitbox topic, it looks like we might have to have our repos migrated
from wherever they are hosted, now, over to Gitbox, before we can do that.
I've got my Github account all linked up with Gitbox, but it shows no
Incubator or Guacamole repos hosted there (that I have access to).  Also,
when I look at the main gitbox.apache.org page with all of the repos, the
Guacamole ones do not show up.

I can open a ticket with INFRA and see what has to happen to do this, if
that's the route we want to go?

-Nick

Re: [DISCUSS] Stalled PRs

[Mike Jumper <mi...@guac-dev.org>]

On Mon, Oct 16, 2017 at 7:47 PM, Nick Couchman <vn...@apache.org> wrote:

> On Sat, Sep 30, 2017 at 3:38 PM, Nick Couchman <vn...@apache.org> wrote:
> >
> >
> > I did track down e-mail address (via JIRA) for the two folks with the
> > aforementioned PRs and e-mailed them directly to see if they could finish
> > up the PRs.  I have not heard anything back from either of them.  Will
> give
> > them a little longer to respond, but I think these two PRs may need to be
> > co-opted and closed out.  Hopefully not - I really would like to see more
> > people actively contributing to the project.
> >
> > -Nick
> >
> No response from either author or either of those pull requests.
>
>
Alright, then. Guess there's nothing to do now but close the pull requests.
If it's possible to use ASF's deployment of Git Box (I don't know what's
required here), then we can do this ourselves. Otherwise, might need to
open a ticket with infra.

- Mike