You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by Volkan Yazıcı <vo...@gmail.com> on 2021/02/03 14:17:49 UTC

Procedure on accepting GitHub PRs

Hello,

Recently we have requested a signed ICLA from Tim Perry for his work
available at GitHub PR #463
<https://github.com/apache/logging-log4j2/pull/463> addressing LOG4J2-1606
and LOG4J2-2624. That said, there have been occasions of me merging PRs
from individuals without requesting a signed ICLA, e.g., #462
<https://github.com/apache/logging-log4j2/pull/462>, #461
<https://github.com/apache/logging-log4j2/pull/461>, #436
<https://github.com/apache/logging-log4j2/pull/436>. What is the official
procedure on this? Are we supposed to not accept any PR without an ICLA
first? What if we/I did?

Kind regards.

Re: Procedure on accepting GitHub PRs

Posted by Matt Sicker <bo...@gmail.com>.
I'd just try to be careful about introducing unnecessary friction for
small contributions. I've never contributed to the FSF or GNU despite
being a big fan of their philosophy due to the even more complicated
paperwork required to submit anything to them (copyright assignment).

On Wed, 3 Feb 2021 at 09:43, Gary Gregory <ga...@gmail.com> wrote:
>
> I wonder if we could give ourselves guidelines like:
>
> - any change to an algorithm is not trivial (I know, i know, every line of
> code can be considered an algorithm)
> - any change created by a tool is trivial, like clean ups that remove
> trailing whitespace, reflectors code and so on. The issue is that the PR
> comment might not say that a tool was used.
>
> I wonder if CYA would simply default us to asking for the paperwork. Most
> projects backed by for profit corporations do that.
>
> Gary
>
> On Wed, Feb 3, 2021, 10:28 Matt Sicker <bo...@gmail.com> wrote:
>
> > It has more to do with copyright. Trivial changes aren't copyrightable
> > or can be trivially verified to be public domain or similar. Think
> > things like typos, one-liners, etc.
> >
> > If you merge a PR from someone without an ICLA, basically, you're
> > taking the responsibility of saying "I verified this code has been
> > legally contributed to us under the correct license". Having an ICLA
> > on file from the contributor moves that responsibility back to them
> > for clearing their own contributions.
> >
> > On Wed, 3 Feb 2021 at 09:10, Volkan Yazıcı <vo...@gmail.com>
> > wrote:
> > >
> > > Would you briefly define *non-trivial changes*, please? I can imagine it
> > > might be difficult to come up with a precise definition, though I would
> > > like to hear one. Is it measured by LoC changes? If so, that is a pretty
> > > objective criteria. Is it measured by the impact? If so, that is a pretty
> > > subjective criteria.
> > >
> > > On Wed, Feb 3, 2021 at 3:53 PM Matt Sicker <bo...@gmail.com> wrote:
> > >
> > > > ICLAs are needed from people who contribute non-trivial changes and
> > > > haven't already filled one out. There's a list of names who already
> > > > submitted ICLAs on this page:
> > > > https://home.apache.org/unlistedclas.html
> > > >
> > > > On Wed, 3 Feb 2021 at 08:17, Volkan Yazıcı <vo...@gmail.com>
> > > > wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > Recently we have requested a signed ICLA from Tim Perry for his work
> > > > > available at GitHub PR #463
> > > > > <https://github.com/apache/logging-log4j2/pull/463> addressing
> > > > LOG4J2-1606
> > > > > and LOG4J2-2624. That said, there have been occasions of me merging
> > PRs
> > > > > from individuals without requesting a signed ICLA, e.g., #462
> > > > > <https://github.com/apache/logging-log4j2/pull/462>, #461
> > > > > <https://github.com/apache/logging-log4j2/pull/461>, #436
> > > > > <https://github.com/apache/logging-log4j2/pull/436>. What is the
> > > > official
> > > > > procedure on this? Are we supposed to not accept any PR without an
> > ICLA
> > > > > first? What if we/I did?
> > > > >
> > > > > Kind regards.
> > > >
> >

Re: Procedure on accepting GitHub PRs

Posted by Gary Gregory <ga...@gmail.com>.
I wonder if we could give ourselves guidelines like:

- any change to an algorithm is not trivial (I know, i know, every line of
code can be considered an algorithm)
- any change created by a tool is trivial, like clean ups that remove
trailing whitespace, reflectors code and so on. The issue is that the PR
comment might not say that a tool was used.

I wonder if CYA would simply default us to asking for the paperwork. Most
projects backed by for profit corporations do that.

Gary

On Wed, Feb 3, 2021, 10:28 Matt Sicker <bo...@gmail.com> wrote:

> It has more to do with copyright. Trivial changes aren't copyrightable
> or can be trivially verified to be public domain or similar. Think
> things like typos, one-liners, etc.
>
> If you merge a PR from someone without an ICLA, basically, you're
> taking the responsibility of saying "I verified this code has been
> legally contributed to us under the correct license". Having an ICLA
> on file from the contributor moves that responsibility back to them
> for clearing their own contributions.
>
> On Wed, 3 Feb 2021 at 09:10, Volkan Yazıcı <vo...@gmail.com>
> wrote:
> >
> > Would you briefly define *non-trivial changes*, please? I can imagine it
> > might be difficult to come up with a precise definition, though I would
> > like to hear one. Is it measured by LoC changes? If so, that is a pretty
> > objective criteria. Is it measured by the impact? If so, that is a pretty
> > subjective criteria.
> >
> > On Wed, Feb 3, 2021 at 3:53 PM Matt Sicker <bo...@gmail.com> wrote:
> >
> > > ICLAs are needed from people who contribute non-trivial changes and
> > > haven't already filled one out. There's a list of names who already
> > > submitted ICLAs on this page:
> > > https://home.apache.org/unlistedclas.html
> > >
> > > On Wed, 3 Feb 2021 at 08:17, Volkan Yazıcı <vo...@gmail.com>
> > > wrote:
> > > >
> > > > Hello,
> > > >
> > > > Recently we have requested a signed ICLA from Tim Perry for his work
> > > > available at GitHub PR #463
> > > > <https://github.com/apache/logging-log4j2/pull/463> addressing
> > > LOG4J2-1606
> > > > and LOG4J2-2624. That said, there have been occasions of me merging
> PRs
> > > > from individuals without requesting a signed ICLA, e.g., #462
> > > > <https://github.com/apache/logging-log4j2/pull/462>, #461
> > > > <https://github.com/apache/logging-log4j2/pull/461>, #436
> > > > <https://github.com/apache/logging-log4j2/pull/436>. What is the
> > > official
> > > > procedure on this? Are we supposed to not accept any PR without an
> ICLA
> > > > first? What if we/I did?
> > > >
> > > > Kind regards.
> > >
>

Re: Procedure on accepting GitHub PRs

Posted by Matt Sicker <bo...@gmail.com>.
It has more to do with copyright. Trivial changes aren't copyrightable
or can be trivially verified to be public domain or similar. Think
things like typos, one-liners, etc.

If you merge a PR from someone without an ICLA, basically, you're
taking the responsibility of saying "I verified this code has been
legally contributed to us under the correct license". Having an ICLA
on file from the contributor moves that responsibility back to them
for clearing their own contributions.

On Wed, 3 Feb 2021 at 09:10, Volkan Yazıcı <vo...@gmail.com> wrote:
>
> Would you briefly define *non-trivial changes*, please? I can imagine it
> might be difficult to come up with a precise definition, though I would
> like to hear one. Is it measured by LoC changes? If so, that is a pretty
> objective criteria. Is it measured by the impact? If so, that is a pretty
> subjective criteria.
>
> On Wed, Feb 3, 2021 at 3:53 PM Matt Sicker <bo...@gmail.com> wrote:
>
> > ICLAs are needed from people who contribute non-trivial changes and
> > haven't already filled one out. There's a list of names who already
> > submitted ICLAs on this page:
> > https://home.apache.org/unlistedclas.html
> >
> > On Wed, 3 Feb 2021 at 08:17, Volkan Yazıcı <vo...@gmail.com>
> > wrote:
> > >
> > > Hello,
> > >
> > > Recently we have requested a signed ICLA from Tim Perry for his work
> > > available at GitHub PR #463
> > > <https://github.com/apache/logging-log4j2/pull/463> addressing
> > LOG4J2-1606
> > > and LOG4J2-2624. That said, there have been occasions of me merging PRs
> > > from individuals without requesting a signed ICLA, e.g., #462
> > > <https://github.com/apache/logging-log4j2/pull/462>, #461
> > > <https://github.com/apache/logging-log4j2/pull/461>, #436
> > > <https://github.com/apache/logging-log4j2/pull/436>. What is the
> > official
> > > procedure on this? Are we supposed to not accept any PR without an ICLA
> > > first? What if we/I did?
> > >
> > > Kind regards.
> >

Re: Procedure on accepting GitHub PRs

Posted by Ralph Goers <ra...@dslextreme.com>.
I guess the answer is, “You know it when you see it”. Usually it is pretty obvious. I wouldn’t go solely by lines of code, but that could be a factor.  

I think the way I would view it is if you look at the PR and can review it in a couple of minutes and say to yourself, “Gee, that looks pretty simple”, then it is probably a trivial change. 

The only harm in asking for an ICLA is if the contributor refuses to submit one.  But remember, since you are merging the commit you are essentially assuring that the person has the right to commit the patch. With an ICLA on file the person is making that assertion, not you. So the bottom line is, are you comfortable making that assertion?

Ralph

> On Feb 3, 2021, at 8:10 AM, Volkan Yazıcı <vo...@gmail.com> wrote:
> 
> Would you briefly define *non-trivial changes*, please? I can imagine it
> might be difficult to come up with a precise definition, though I would
> like to hear one. Is it measured by LoC changes? If so, that is a pretty
> objective criteria. Is it measured by the impact? If so, that is a pretty
> subjective criteria.
> 
> On Wed, Feb 3, 2021 at 3:53 PM Matt Sicker <bo...@gmail.com> wrote:
> 
>> ICLAs are needed from people who contribute non-trivial changes and
>> haven't already filled one out. There's a list of names who already
>> submitted ICLAs on this page:
>> https://home.apache.org/unlistedclas.html
>> 
>> On Wed, 3 Feb 2021 at 08:17, Volkan Yazıcı <vo...@gmail.com>
>> wrote:
>>> 
>>> Hello,
>>> 
>>> Recently we have requested a signed ICLA from Tim Perry for his work
>>> available at GitHub PR #463
>>> <https://github.com/apache/logging-log4j2/pull/463> addressing
>> LOG4J2-1606
>>> and LOG4J2-2624. That said, there have been occasions of me merging PRs
>>> from individuals without requesting a signed ICLA, e.g., #462
>>> <https://github.com/apache/logging-log4j2/pull/462>, #461
>>> <https://github.com/apache/logging-log4j2/pull/461>, #436
>>> <https://github.com/apache/logging-log4j2/pull/436>. What is the
>> official
>>> procedure on this? Are we supposed to not accept any PR without an ICLA
>>> first? What if we/I did?
>>> 
>>> Kind regards.
>> 



Re: Procedure on accepting GitHub PRs

Posted by Volkan Yazıcı <vo...@gmail.com>.
Would you briefly define *non-trivial changes*, please? I can imagine it
might be difficult to come up with a precise definition, though I would
like to hear one. Is it measured by LoC changes? If so, that is a pretty
objective criteria. Is it measured by the impact? If so, that is a pretty
subjective criteria.

On Wed, Feb 3, 2021 at 3:53 PM Matt Sicker <bo...@gmail.com> wrote:

> ICLAs are needed from people who contribute non-trivial changes and
> haven't already filled one out. There's a list of names who already
> submitted ICLAs on this page:
> https://home.apache.org/unlistedclas.html
>
> On Wed, 3 Feb 2021 at 08:17, Volkan Yazıcı <vo...@gmail.com>
> wrote:
> >
> > Hello,
> >
> > Recently we have requested a signed ICLA from Tim Perry for his work
> > available at GitHub PR #463
> > <https://github.com/apache/logging-log4j2/pull/463> addressing
> LOG4J2-1606
> > and LOG4J2-2624. That said, there have been occasions of me merging PRs
> > from individuals without requesting a signed ICLA, e.g., #462
> > <https://github.com/apache/logging-log4j2/pull/462>, #461
> > <https://github.com/apache/logging-log4j2/pull/461>, #436
> > <https://github.com/apache/logging-log4j2/pull/436>. What is the
> official
> > procedure on this? Are we supposed to not accept any PR without an ICLA
> > first? What if we/I did?
> >
> > Kind regards.
>

Re: Procedure on accepting GitHub PRs

Posted by Matt Sicker <bo...@gmail.com>.
ICLAs are needed from people who contribute non-trivial changes and
haven't already filled one out. There's a list of names who already
submitted ICLAs on this page:
https://home.apache.org/unlistedclas.html

On Wed, 3 Feb 2021 at 08:17, Volkan Yazıcı <vo...@gmail.com> wrote:
>
> Hello,
>
> Recently we have requested a signed ICLA from Tim Perry for his work
> available at GitHub PR #463
> <https://github.com/apache/logging-log4j2/pull/463> addressing LOG4J2-1606
> and LOG4J2-2624. That said, there have been occasions of me merging PRs
> from individuals without requesting a signed ICLA, e.g., #462
> <https://github.com/apache/logging-log4j2/pull/462>, #461
> <https://github.com/apache/logging-log4j2/pull/461>, #436
> <https://github.com/apache/logging-log4j2/pull/436>. What is the official
> procedure on this? Are we supposed to not accept any PR without an ICLA
> first? What if we/I did?
>
> Kind regards.