How to recreate Github squash commit message Co-authored-by trailer with commit template?

[melink14]

Hi all,

My current config uses squash-and-merge action with custom commit message template to use PR title and body as commit message. The main reason for this is that commits added during the review process are useful snapshots for discussion but the PR itself should always be a single atomic unit of change that is defined by title and body. It's also much easier to change than the commit messages themselves without mutating git history.

Github's default squash commit unfortunately concatenates all commit messges in this case which is dilutes the meaning of the PR.

What is nice though is that Github pre-populates Co-authored-by tags with all the commit authors being squashed which preserves history and more accurately reflects the contributors to the final commit.

From what I can tell, there's no commit author attribute to reference in the commit message template (like there is for approvers for example) but maybe I'm missing a way I can't see now.

I think otherwise probably the best thing to do is to manually add Co-authored-by: to the body of the pull request manually when necessary.

Thanks!

[jd]

We don't expose a list of all commit authors in the attribute, but I imagine that'd solve it.

It'd be nice to have commits as a list of commits and refer to c.author for c in commits or the like.

That's something to add on the roadmap for sure. :)

[jd]

Note: that this has been added a few months ago.

[thomaseizinger]

One thing to note is that Co-authored-by needs to be the last thing in the commit message, otherwise Github doesn't recognize it.

I am currently testing the following template:

{{ title }}

{{ body | get_section("## Description", "") }}

Pull-Request: #{{ number }}.

{# Here comes some fancy Jinja2 stuff for correctly attributing co-authorship: #}

{% set unique_authors = {} %}
{% for commit in commits %}
  {% set should_add_coauthor = true %}

  {# Merge commits should not add attribution #}
  {% if commit.parents|length > 1 %}
    {% set should_add_coauthor = false %}
  {% endif %}

  {# The original author will already be attributed by GitHub #}
  {% if commit.author == pull_request.user.login %}
    {% set should_add_coauthor = false %}
  {% endif %}

  {# Only attribute each author once #}
  {% if unique_authors.get(commit.email_author) %}
    {% set should_add_coauthor = false %}
  {% endif %}

  {% if should_add_coauthor %}
    Co-Authored-By: {{ commit.author }} <{{ commit.email_author }}>
  {% endif %}

  {% set _ = unique_authors.update({commit.email_author: true}) %}
{% endfor %}

{# GitHub requires that the `Co-authored-by` lines are AT THE VERY END of a commit, hence nothing must come after this. #}

[thomaseizinger]

Yes, we squash merge commits and want to attribute all contributors of a PR, excluding merge commits.

Here is my final version:

        {# Here comes some fancy Jinja2 stuff for correctly attributing co-authorship: #}
        {% set co_authored_by = {} %}
        {# We exclude merge commits (> 1 parent), commits from the PR author and only want to render one `Co-Authored-By` line per contributor. #}
        {% for commit in commits
            if  commit.parents|length <= 1 
            and commit.author != author
            and not co_authored_by.get(commit.email_author) %}
          {% set _ = co_authored_by.update({commit.email_author: true}) %}
        Co-Authored-By: {{ commit.author }} <{{ commit.email_author }}>
        {% endfor %}
        {# GitHub requires that the `Co-authored-by` lines are AT THE VERY END of a commit, hence nothing must come after this. #}

[thomaseizinger]

The most annoying bit about this logic is avoiding duplicate lines. I am not too familiar with Python and Jinja2 but maybe adding some filters would be sufficient? Something like commits | unique_by_author_email?

[thomaseizinger]

Hang on, that actually exists! https://jinja.palletsprojects.com/en/3.0.x/templates/#jinja-filters.unique

[thomaseizinger]

I think I was just able to simplify this down to:

{# Here comes some fancy Jinja2 stuff for correctly attributing co-authorship: #}
{% for commit in (commits | unique(False, 'email_author')) | rejectattr("author", "==", author) %}
  {% if commit.parents|length == 1 %}
Co-Authored-By: {{ commit.author }} <{{ commit.email_author }}>
  {% endif %}
{% endfor %}
{# GitHub requires that the `Co-authored-by` lines are AT THE VERY END of a commit, hence nothing must come after this. #}

[thomaseizinger]

What would be nice if we could get rid of the final if and replace it with a filter. Unfortunately, testing for length isn't really supported but I imagine you could add it?

For example:

{# Here comes some fancy Jinja2 stuff for correctly attributing co-authorship: #}
{% for commit in commits | unique(False, 'email_author') | rejectattr("author", "==", author) | reject("mergecommit") %}
Co-Authored-By: {{ commit.author }} <{{ commit.email_author }}>
{% endfor %}
{# GitHub requires that the `Co-authored-by` lines are AT THE VERY END of a commit, hence nothing must come after this. #}

[jd]

That's neat. I think if we go down on simplifying, we'll probably expose the list of co-authors of a PR as an attribute.

Something like:

{% for co_author in co_authors %}
Co-Authored-By: {{ co_author.name }} <{{ co_author.email }}>
{% endfor %}

I've made a note to document this and add the attribute.

[thomaseizinger]

That would still require filtering for duplicates because it is attached to the commits, right? If you wanted to add a new attribute, I think it would have to be at the top-level:

{% for co_author in co_authors %}
Co-Authored-By: {{ co_author.name }} <{{ co_author.email }}>
{% endfor %}

[jd]

You're right, I've fixed my example!

[thomaseizinger]

This seems to now be implemented as I've found the following in the docs: https://docs.mergify.com/configuration/#data-type-commit-author

Can you confirm that merge commits are excluded from that?

[jd]

Only PR author is ignored AFAICT. I'll check if we can ignore merge author also.

[DouglasBlackwood]

co-authors is the list of commit authors without the author of the pull request. If the pull request contains merge commits, they are not excluded. Can you give a try on that new attribute and give us feedback? If it doesn't work as expected, it could be great to share an example.

[thomaseizinger]

co-authors is the list of commit authors without the author of the pull request. If the pull request contains merge commits, they are not excluded. Can you give a try on that new attribute and give us feedback? If it doesn't work as expected, it could be great to share an example.

Please have a look at the other answer and its replies for a more detailed discussion.

Excluding merge commits is important. They aren't contributions themselves and happen regularly when updating PRs. Merify itself makes many merge commits and it IMO doesn't make sense to add it as a "co-author" to each squash-merged PR.

"Giving this a try" is a fair amount of work as it requires the coordination of multiple user accounts, plus I don't really want to merge "dummy" PRs into our main branch just to test that this works as expected.

As described elsewhere, my expectation for this feature is:

• Merge commits are excluded
• Commits from PR author are excluded
• Co-authors is a set of unique entries (i.e. no duplicates)
• A co-author that has pushed merge commits and other commits must still show up as a co-author

The last requirement is what makes this difficult / impossible to implement as a Jinja2 template.

It would be great if just iterating over the co-authors will do the right thing :)

Side-note: What would make iteration on such templates a lot easier is if the config editor could expand the template and show what the commit message would look like. Commit trailers are extremely white-space sensitive so having to iterate on this by merging PRs is very time-consuming.