ci: support co-authoring for auto-merged PRs #176

smoya · 2022-09-13T19:49:18Z

Description

Something I realized we might need to think about is the fact merging release branches (i.e. next-major or next-spec) with squash merging strategy ends up obviously with only one commit being merged, being the author of it the person who created the PR.

This leads to:

Losing history of changes introduced. It might not be a real issue (if it hasn’t yet) since we write version notes. However, it’s hard to see where exactly a feature or bug was introduced.
Losing authority of commits. All changes to such version will appear to be introduced by the person who created the PR.

For the record, the merge is performed automatically by @asyncapi-bot thanks to the automerge-for-humans-merging.yml Github workflow.

This PR fixes the issue with authority, by adding co-authors to the final commit (the one that ends up being merged). See https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line.

You can see the workflow running on this test repository I created:

PR: docs: remove line smoya/automerge-coauthoring#1
- The PR has two commits, one by a fictitious user jciment, another is made by me; smoya.
  The merged commit: smoya/automerge-coauthoring@7071981
- The commit contains Co-authored-by: Sergio Moya <[email protected]>.
- GH UI shows the co-author as expected:

Examples:

spec 2.4.0:
- PR opened by @sergio Moya: feat: release for version 2.4.0 of the spec spec#740
  - You can notice there are several commits merged into that PR, from several authors.
- The actual merged commit, being @sergio Moya the only author: asyncapi/spec@65d5869

The same will happen with the 2.0.0 version of the parser-js asyncapi/parser-js#598. And same with version 3 of the spec (this time @jonaslagoni will be the author of the whole v3 spec!): asyncapi/spec#759

Related issue(s)
Slack 🧵 https://asyncapi.slack.com/archives/CQVJXFNQL/p1662972475762699

cc @derberg @fmvilas @jonaslagoni @magicmatatjahu

derberg · 2022-09-13T20:01:39Z

.github/workflows/automerge-for-humans-merging.yml

+        id: authors
+        with:
+          cmd: | 
+            curl -H "Accept: application/vnd.github+json" -H "Authorization: Bearer ${{ secrets.GH_TOKEN }}" "${{github.event.pull_request._links.commits.href}}?per_page=100" | jq -r '[.[] | {name: .commit.author.name, email: .commit.author.email, login: .author.login}] | map(select(.login != "${{github.event.pull_request.user.login}}")) | unique | map("Co-authored-by: " + .name + " <" + .email + ">") | join("\n")'


👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀
👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀
👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀
👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀 👀
I'm not sure I can survive it.....need to pick up my brain from the neighborhood and see again...

Life is hard sometimes :( But I can do multiline if you prefer to read like:

👀👀 👀 👀
👀👀 👀 👀
👀👀 👀 👀
👀👀 👀 👀
👀👀 👀 👀
👀👀 👀 👀
👀👀 👀 👀
👀👀 👀 👀
👀👀 👀 👀

Btw, I updated the PR description adding an execution of this change in a new test repository I created. Pasting the addition here for reference:

You can see the workflow running on this test repository I created:

PR: docs: remove line smoya/automerge-coauthoring#1

The PR has two commits, one by a fictitious user jciment, another is made by me; smoya.
The merged commit: smoya/automerge-coauthoring@7071981

The commit contains Co-authored-by: Sergio Moya <[email protected]>.

GH UI shows the co-author as expected:

@derberg I've updated the cmd to become multiline. I hope this helps reviewers to follow what it does. I'm gonna add extra comments on this PR to explain almost line by what it does as well.

smoya · 2022-09-13T21:08:46Z

.github/workflows/automerge-for-humans-merging.yml

+        id: authors
+        with:
+          cmd: | 
+            curl -H "Accept: application/vnd.github+json" -H "Authorization: Bearer ${{ secrets.GH_TOKEN }}" "${{github.event.pull_request._links.commits.href}}?per_page=100" 


This is querying the list of commits of the current PR via GH API. Why? Because the current event payload does not carry info about the commits.

smoya · 2022-09-13T21:10:54Z

.github/workflows/automerge-for-humans-merging.yml

+        with:
+          cmd: | 
+            curl -H "Accept: application/vnd.github+json" -H "Authorization: Bearer ${{ secrets.GH_TOKEN }}" "${{github.event.pull_request._links.commits.href}}?per_page=100" 
+              | jq -r '[.[] 


Iterating over the payload GH gives us, creating an array with the filtered results (below). An example of payload can be found in https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#webhook-payload-example-34

smoya · 2022-09-13T21:12:36Z

.github/workflows/automerge-for-humans-merging.yml

+          cmd: | 
+            curl -H "Accept: application/vnd.github+json" -H "Authorization: Bearer ${{ secrets.GH_TOKEN }}" "${{github.event.pull_request._links.commits.href}}?per_page=100" 
+              | jq -r '[.[] 
+                | {name: .commit.author.name, email: .commit.author.email, login: .author.login}] 


This line grabs the data we need for adding the Co-authored-by: ... lines later and puts it into objects to be used later on.

smoya · 2022-09-13T21:13:33Z

.github/workflows/automerge-for-humans-merging.yml

+            curl -H "Accept: application/vnd.github+json" -H "Authorization: Bearer ${{ secrets.GH_TOKEN }}" "${{github.event.pull_request._links.commits.href}}?per_page=100" 
+              | jq -r '[.[] 
+                | {name: .commit.author.name, email: .commit.author.email, login: .author.login}] 
+                | map(select(.login != "${{github.event.pull_request.user.login}}")) 


This line filters results by excluding the current PR sender. We don't need to add it as co-author since is the PR creator and it will become by default the main author.

smoya · 2022-09-13T21:13:56Z

.github/workflows/automerge-for-humans-merging.yml

+              | jq -r '[.[] 
+                | {name: .commit.author.name, email: .commit.author.email, login: .author.login}] 
+                | map(select(.login != "${{github.event.pull_request.user.login}}")) 
+                | unique 


We remove repeated authors

smoya · 2022-09-13T21:14:20Z

.github/workflows/automerge-for-humans-merging.yml

+                | {name: .commit.author.name, email: .commit.author.email, login: .author.login}] 
+                | map(select(.login != "${{github.event.pull_request.user.login}}")) 
+                | unique 
+                | map("Co-authored-by: " + .name + " <" + .email + ">") 


We finally write the lines we want to later on print to stdout

smoya · 2022-09-13T21:15:04Z

.github/workflows/automerge-for-humans-merging.yml

+                | map(select(.login != "${{github.event.pull_request.user.login}}")) 
+                | unique 
+                | map("Co-authored-by: " + .name + " <" + .email + ">") 
+                | join("\n")'


Transforming the array into a plain text so we can directly copy this stdout to the next Workflow step (wich is basically the automerge)

smoya · 2022-09-13T21:15:29Z

.github/workflows/automerge-for-humans-merging.yml

      - name: Automerge PR
        uses: pascalgn/[email protected]
        env:
          GITHUB_TOKEN: "${{ secrets.GH_TOKEN }}"
          MERGE_LABELS: "!do-not-merge,ready-to-merge"
          MERGE_METHOD: "squash"
-          MERGE_COMMIT_MESSAGE: "{pullRequest.title} (#{pullRequest.number})"
+          # Important to keep 2 empty lines as https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line mentions
+          MERGE_COMMIT_MESSAGE: "{pullRequest.title} (#{pullRequest.number})\n\n\n${{ steps.authors.outputs.value }}" 


We use the output of the previous step and put it into the description.

derberg · 2022-09-15T09:01:11Z

@smoya thanks so much for the comments. Can you turn them into a comment inside the yaml file? I guess they cannot be inline, so just as a big comment for the step?

smoya · 2022-09-15T09:47:13Z

@smoya thanks so much for the comments. Can you turn them into a comment inside the yaml file? I guess they cannot be inline, so just as a big comment for the step?

Good point! Done. Would you mind checking again?

derberg

@smoya LGTM, thanks a 💯 for that one 🙇🏼

derberg · 2022-09-15T20:08:18Z

/rtm

ci: support co-authoring for auto-merged PRs

fedf268

smoya requested review from derberg, quetzalliwrites, fmvilas, KhudaDad414 and asyncapi-bot-eve as code owners September 13, 2022 19:49

derberg reviewed Sep 13, 2022

View reviewed changes

help Lukasz reviewing my PR ♥️

0c24654

smoya commented Sep 13, 2022

View reviewed changes

add comments

45b963a

smoya requested a review from derberg September 15, 2022 09:47

derberg approved these changes Sep 15, 2022

View reviewed changes

asyncapi-bot added the ready-to-merge label Sep 15, 2022

asyncapi-bot merged commit faae49a into asyncapi:master Sep 15, 2022

smoya deleted the ci/coauthoring branch September 15, 2022 20:45

smoya mentioned this pull request Sep 15, 2022

[Feature request] Support PR co-authoring when squashing via commit description pascalgn/automerge-action#196

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: support co-authoring for auto-merged PRs #176

ci: support co-authoring for auto-merged PRs #176

smoya commented Sep 13, 2022 •

edited

Loading

derberg Sep 13, 2022

smoya Sep 13, 2022 •

edited

Loading

smoya Sep 13, 2022

smoya Sep 13, 2022

smoya Sep 13, 2022

smoya Sep 13, 2022

smoya Sep 13, 2022

smoya Sep 13, 2022

smoya Sep 13, 2022

smoya Sep 13, 2022

smoya Sep 13, 2022

smoya Sep 13, 2022

derberg commented Sep 15, 2022

smoya commented Sep 15, 2022

derberg left a comment

derberg commented Sep 15, 2022

ci: support co-authoring for auto-merged PRs #176

ci: support co-authoring for auto-merged PRs #176

Conversation

smoya commented Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

smoya Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derberg commented Sep 15, 2022

smoya commented Sep 15, 2022

derberg left a comment

Choose a reason for hiding this comment

derberg commented Sep 15, 2022

smoya commented Sep 13, 2022 •

edited

Loading

smoya Sep 13, 2022 •

edited

Loading