You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Kevin Gurney <kg...@mathworks.com> on 2021/08/04 14:45:28 UTC

Support for Co-authored-by tag on individual commits when integrating pull requests

Hi All,

Fiona La (Cc'd) and I recently worked together with Kou to integrate some changes to the MATLAB interface (pull request: https://github.com/apache/arrow/pull/10614). Fiona and I pair programmed the implementation together on "one machine", using my GitHub account to push commits. We used GitHub's support for Co-authored-by tags (https://docs.github.com/en/github/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors) to include Fiona's name on every commit. We thought this would be sufficient to ensure that her name was included in the main Apache Arrow git history after the commits were squashed and integrated by Kou. Unfortunately, it looks like her name was dropped from the list of Co-authors during integration.

In order to ensure that all contributors to the project get credit:

  1.  Is there an existing, recommended best practice for pair programming on pull requests that ensures all contributors get credit?
     *   We realized after the pull request was integrated that Fiona may have gotten credit if she pushed at least one commit from a separate GitHub account. Although, we aren't 100% sure if this true.
  2.  It looks like https://github.com/apache/arrow/blob/master/dev/merge_arrow_pr.py does not support the Co-authored-by tag workflow on individual commits described above.
     *   We are interested in opening a pull request to modify merge_arrow_pr.py to add support for this workflow.
  3.  Is there a way to retroactively add Fiona's name to the git history for https://github.com/apache/arrow/pull/10614 so she receives credit?

Thank you!

Kevin Gurney

Re: Support for Co-authored-by tag on individual commits when integrating pull requests

Posted by Sutou Kouhei <ko...@clear-code.com>.
Hi Kevin and Fiona,

Sorry for not noticing it on the merge and thanks for
opening a JIRA issue for this. Please ping me on GitHub when
a pull request for the issue is created. I can review it.


Thanks,
-- 
kou

In <MN...@MN2PR05MB7022.namprd05.prod.outlook.com>
  "Re: Support for Co-authored-by tag on individual commits when integrating pull requests" on Wed, 4 Aug 2021 20:11:38 +0000,
  Fiona La <fi...@mathworks.com> wrote:

> Thanks Wes and Kevin!
> 
> I have opened a Jira ticket for tracking this work: https://issues.apache.org/jira/browse/ARROW-13564.
> 
> Regards,
> Fiona
> 
> From: Kevin Gurney <kg...@mathworks.com>
> Date: Wednesday, August 4, 2021 at 4:00 PM
> To: dev <de...@arrow.apache.org>
> Cc: Fiona La <fi...@mathworks.com>
> Subject: Re: Support for Co-authored-by tag on individual commits when integrating pull requests
> Hi Wes,
> 
> Thank you for the quick response!
> 
> No need to apologize! The Co-authored-by workflow is new to us, so we are learning what works as we go.
> 
> In terms of adding Fiona's name to the pull request that's already been integrated, we appreciate your consideration, but understand if this is too difficult to fix in the main branch at this point.
> 
> To prevent this issue from occurring in the future, we will open a pull request to modify the merge_arrow_pr.py script to scrape "Co-authored-by" tags as suggested.
> 
> Thank you!
> 
> Kevin
> 
> ________________________________
> From: Wes McKinney <we...@gmail.com>
> Sent: Wednesday, August 4, 2021 11:02 AM
> To: dev <de...@arrow.apache.org>
> Cc: Fiona La <fi...@mathworks.com>
> Subject: Re: Support for Co-authored-by tag on individual commits when integrating pull requests
> 
> hi Kevin,
> 
> Unfortunately, I don't think it's possible to amend the existing
> commit logs because that would require force-pushing the main branch.
> I suppose we could revert the commit and push a new commit with the
> commit message fixed.
> 
>> We realized after the pull request was integrated that Fiona may have gotten credit if she pushed at least one commit from a separate GitHub account. Although, we aren't 100% sure if this true.
> 
> Indeed, if Fiona's e-mail address was in the git Author field for any
> commit in the PR, the PR merge script would have added a
> "Co-authored-by:" message to the squashed commit message.
> 
> I think the next step here is to modify the PR merge script to scrape
> any "Co-authored-by:" lines from the individual commit messages so
> they can all be listed in the combined PR message.
> 
> Sorry about this, this is the first incidence of this particular issue
> occurring to my knowledge.
> 
> Thanks
> Wes
> 
> On Wed, Aug 4, 2021 at 9:46 AM Kevin Gurney <kg...@mathworks.com> wrote:
>>
>> Hi All,
>>
>> Fiona La (Cc'd) and I recently worked together with Kou to integrate some changes to the MATLAB interface (pull request: https://github.com/apache/arrow/pull/10614<https://github.com/apache/arrow/pull/10614>). Fiona and I pair programmed the implementation together on "one machine", using my GitHub account to push commits. We used GitHub's support for Co-authored-by tags (https://docs.github.com/en/github/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors<https://docs.github.com/en/github/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors>) to include Fiona's name on every commit. We thought this would be sufficient to ensure that her name was included in the main Apache Arrow git history after the commits were squashed and integrated by Kou. Unfortunately, it looks like her name was dropped from the list of Co-authors during integration.
>>
>> In order to ensure that all contributors to the project get credit:
>>
>> 1. Is there an existing, recommended best practice for pair programming on pull requests that ensures all contributors get credit?
>> * We realized after the pull request was integrated that Fiona may have gotten credit if she pushed at least one commit from a separate GitHub account. Although, we aren't 100% sure if this true.
>> 2. It looks like https://github.com/apache/arrow/blob/master/dev/merge_arrow_pr.py<https://github.com/apache/arrow/blob/master/dev/merge_arrow_pr.py> does not support the Co-authored-by tag workflow on individual commits described above.
>> * We are interested in opening a pull request to modify merge_arrow_pr.py to add support for this workflow.
>> 3. Is there a way to retroactively add Fiona's name to the git history for https://github.com/apache/arrow/pull/10614<https://github.com/apache/arrow/pull/10614> so she receives credit?
>>
>> Thank you!
>>
>> Kevin Gurney

Re: Support for Co-authored-by tag on individual commits when integrating pull requests

Posted by Fiona La <fi...@mathworks.com>.
Thanks Wes and Kevin!

I have opened a Jira ticket for tracking this work: https://issues.apache.org/jira/browse/ARROW-13564.

Regards,
Fiona

From: Kevin Gurney <kg...@mathworks.com>
Date: Wednesday, August 4, 2021 at 4:00 PM
To: dev <de...@arrow.apache.org>
Cc: Fiona La <fi...@mathworks.com>
Subject: Re: Support for Co-authored-by tag on individual commits when integrating pull requests
Hi Wes,

Thank you for the quick response!

No need to apologize! The Co-authored-by workflow is new to us, so we are learning what works as we go.

In terms of adding Fiona's name to the pull request that's already been integrated, we appreciate your consideration, but understand if this is too difficult to fix in the main branch at this point.

To prevent this issue from occurring in the future, we will open a pull request to modify the merge_arrow_pr.py script to scrape "Co-authored-by" tags as suggested.

Thank you!

Kevin

________________________________
From: Wes McKinney <we...@gmail.com>
Sent: Wednesday, August 4, 2021 11:02 AM
To: dev <de...@arrow.apache.org>
Cc: Fiona La <fi...@mathworks.com>
Subject: Re: Support for Co-authored-by tag on individual commits when integrating pull requests

hi Kevin,

Unfortunately, I don't think it's possible to amend the existing
commit logs because that would require force-pushing the main branch.
I suppose we could revert the commit and push a new commit with the
commit message fixed.

> We realized after the pull request was integrated that Fiona may have gotten credit if she pushed at least one commit from a separate GitHub account. Although, we aren't 100% sure if this true.

Indeed, if Fiona's e-mail address was in the git Author field for any
commit in the PR, the PR merge script would have added a
"Co-authored-by:" message to the squashed commit message.

I think the next step here is to modify the PR merge script to scrape
any "Co-authored-by:" lines from the individual commit messages so
they can all be listed in the combined PR message.

Sorry about this, this is the first incidence of this particular issue
occurring to my knowledge.

Thanks
Wes

On Wed, Aug 4, 2021 at 9:46 AM Kevin Gurney <kg...@mathworks.com> wrote:
>
> Hi All,
>
> Fiona La (Cc'd) and I recently worked together with Kou to integrate some changes to the MATLAB interface (pull request: https://github.com/apache/arrow/pull/10614<https://github.com/apache/arrow/pull/10614>). Fiona and I pair programmed the implementation together on "one machine", using my GitHub account to push commits. We used GitHub's support for Co-authored-by tags (https://docs.github.com/en/github/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors<https://docs.github.com/en/github/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors>) to include Fiona's name on every commit. We thought this would be sufficient to ensure that her name was included in the main Apache Arrow git history after the commits were squashed and integrated by Kou. Unfortunately, it looks like her name was dropped from the list of Co-authors during integration.
>
> In order to ensure that all contributors to the project get credit:
>
> 1. Is there an existing, recommended best practice for pair programming on pull requests that ensures all contributors get credit?
> * We realized after the pull request was integrated that Fiona may have gotten credit if she pushed at least one commit from a separate GitHub account. Although, we aren't 100% sure if this true.
> 2. It looks like https://github.com/apache/arrow/blob/master/dev/merge_arrow_pr.py<https://github.com/apache/arrow/blob/master/dev/merge_arrow_pr.py> does not support the Co-authored-by tag workflow on individual commits described above.
> * We are interested in opening a pull request to modify merge_arrow_pr.py to add support for this workflow.
> 3. Is there a way to retroactively add Fiona's name to the git history for https://github.com/apache/arrow/pull/10614<https://github.com/apache/arrow/pull/10614> so she receives credit?
>
> Thank you!
>
> Kevin Gurney

Re: Support for Co-authored-by tag on individual commits when integrating pull requests

Posted by Kevin Gurney <kg...@mathworks.com>.
Hi Wes,

Thank you for the quick response!

No need to apologize! The Co-authored-by workflow is new to us, so we are learning what works as we go.

In terms of adding Fiona's name to the pull request that's already been integrated, we appreciate your consideration, but understand if this is too difficult to fix in the main branch at this point.

To prevent this issue from occurring in the future, we will open a pull request to modify the merge_arrow_pr.py script to scrape "Co-authored-by" tags as suggested.

Thank you!

Kevin

________________________________
From: Wes McKinney <we...@gmail.com>
Sent: Wednesday, August 4, 2021 11:02 AM
To: dev <de...@arrow.apache.org>
Cc: Fiona La <fi...@mathworks.com>
Subject: Re: Support for Co-authored-by tag on individual commits when integrating pull requests

hi Kevin,

Unfortunately, I don't think it's possible to amend the existing
commit logs because that would require force-pushing the main branch.
I suppose we could revert the commit and push a new commit with the
commit message fixed.

> We realized after the pull request was integrated that Fiona may have gotten credit if she pushed at least one commit from a separate GitHub account. Although, we aren't 100% sure if this true.

Indeed, if Fiona's e-mail address was in the git Author field for any
commit in the PR, the PR merge script would have added a
"Co-authored-by:" message to the squashed commit message.

I think the next step here is to modify the PR merge script to scrape
any "Co-authored-by:" lines from the individual commit messages so
they can all be listed in the combined PR message.

Sorry about this, this is the first incidence of this particular issue
occurring to my knowledge.

Thanks
Wes

On Wed, Aug 4, 2021 at 9:46 AM Kevin Gurney <kg...@mathworks.com> wrote:
>
> Hi All,
>
> Fiona La (Cc'd) and I recently worked together with Kou to integrate some changes to the MATLAB interface (pull request: https://github.com/apache/arrow/pull/10614<https://github.com/apache/arrow/pull/10614>). Fiona and I pair programmed the implementation together on "one machine", using my GitHub account to push commits. We used GitHub's support for Co-authored-by tags (https://docs.github.com/en/github/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors<https://docs.github.com/en/github/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors>) to include Fiona's name on every commit. We thought this would be sufficient to ensure that her name was included in the main Apache Arrow git history after the commits were squashed and integrated by Kou. Unfortunately, it looks like her name was dropped from the list of Co-authors during integration.
>
> In order to ensure that all contributors to the project get credit:
>
> 1. Is there an existing, recommended best practice for pair programming on pull requests that ensures all contributors get credit?
> * We realized after the pull request was integrated that Fiona may have gotten credit if she pushed at least one commit from a separate GitHub account. Although, we aren't 100% sure if this true.
> 2. It looks like https://github.com/apache/arrow/blob/master/dev/merge_arrow_pr.py<https://github.com/apache/arrow/blob/master/dev/merge_arrow_pr.py> does not support the Co-authored-by tag workflow on individual commits described above.
> * We are interested in opening a pull request to modify merge_arrow_pr.py to add support for this workflow.
> 3. Is there a way to retroactively add Fiona's name to the git history for https://github.com/apache/arrow/pull/10614<https://github.com/apache/arrow/pull/10614> so she receives credit?
>
> Thank you!
>
> Kevin Gurney

Re: Support for Co-authored-by tag on individual commits when integrating pull requests

Posted by Wes McKinney <we...@gmail.com>.
hi Kevin,

Unfortunately, I don't think it's possible to amend the existing
commit logs because that would require force-pushing the main branch.
I suppose we could revert the commit and push a new commit with the
commit message fixed.

> We realized after the pull request was integrated that Fiona may have gotten credit if she pushed at least one commit from a separate GitHub account. Although, we aren't 100% sure if this true.

Indeed, if Fiona's e-mail address was in the git Author field for any
commit in the PR, the PR merge script would have added a
"Co-authored-by:" message to the squashed commit message.

I think the next step here is to modify the PR merge script to scrape
any "Co-authored-by:" lines from the individual commit messages so
they can all be listed in the combined PR message.

Sorry about this, this is the first incidence of this particular issue
occurring to my knowledge.

Thanks
Wes

On Wed, Aug 4, 2021 at 9:46 AM Kevin Gurney <kg...@mathworks.com> wrote:
>
> Hi All,
>
> Fiona La (Cc'd) and I recently worked together with Kou to integrate some changes to the MATLAB interface (pull request: https://github.com/apache/arrow/pull/10614). Fiona and I pair programmed the implementation together on "one machine", using my GitHub account to push commits. We used GitHub's support for Co-authored-by tags (https://docs.github.com/en/github/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors) to include Fiona's name on every commit. We thought this would be sufficient to ensure that her name was included in the main Apache Arrow git history after the commits were squashed and integrated by Kou. Unfortunately, it looks like her name was dropped from the list of Co-authors during integration.
>
> In order to ensure that all contributors to the project get credit:
>
>   1.  Is there an existing, recommended best practice for pair programming on pull requests that ensures all contributors get credit?
>      *   We realized after the pull request was integrated that Fiona may have gotten credit if she pushed at least one commit from a separate GitHub account. Although, we aren't 100% sure if this true.
>   2.  It looks like https://github.com/apache/arrow/blob/master/dev/merge_arrow_pr.py does not support the Co-authored-by tag workflow on individual commits described above.
>      *   We are interested in opening a pull request to modify merge_arrow_pr.py to add support for this workflow.
>   3.  Is there a way to retroactively add Fiona's name to the git history for https://github.com/apache/arrow/pull/10614 so she receives credit?
>
> Thank you!
>
> Kevin Gurney