Misc #14470: Use Commit together with co-authors Github feature in svn commits

• Status: Closed
• Assignee: -

[ana06 (Ana Maria Martinez Gomez)]

As currently Ruby is using SVN, for "merging" a PR, the PR is closed and the changes added in a SVN commit. In this commit the author is someone from the Ruby core and not the person who wrote the code. The person who wrote the code is mentioned in the commit message, but that doesn't give any visibility to the author of the code. I think giving proper attribution to the contributors of the projects is important to value them.

It seems that any solution was found to solve this problem, but I think that it could be now easily solved with the new Commit together with co-authors Github feature.

If in the commit message, instead of

Submitted by: @Ana06 <[e-mail]>

you write

Co-authored-by: Ana María Martínez Gómez <[e-mail]>

Github will show the contribution as well.

What do you think? Would you be up to use it? I think it would be a really easy way to give contributors more visibility. 😉

There have been already some discussions in https://github.com/ruby/ruby/pull/1752

[shevegen (Robert A. Heiler)]

I think the ruby core team and/or matz has to decide on attribution.
It can probably be done in SVN as-is, without depending on git/github.

I was a bit confused about the initial wording of the suggestion,
but then I looked at the link, and it was contribution e. g.
documentation, so I understand if some people may want to have
better attribution of work/time investment there, if they want
to. (I personally don't mind either way, but I can understand if
some others, for whatever reason, may want to have more
attribution for anything that is a non-trivial addition. E. g.
typo corrections ... would these need attribution? That would
seem a bit overkill. But it's fine for improving documentation,
since that will also help other people who learn/use ruby)

[nobu (Nobuyoshi Nakada)]

Why doesn't Github see From: header too?

[nobu (Nobuyoshi Nakada)]

The feature page says "Include your trailers at the end of your commit message", does it work even if "git-svn-id:" line is appended by git svn dcommit?

[tenderlovemaking (Aaron Patterson)]

nobu (Nobuyoshi Nakada) wrote:

The feature page says "Include your trailers at the end of your commit message", does it work even if "git-svn-id:" line is appended by git svn dcommit?

It will work, but we have to ensure there is no newline between the git-svn-id trailer and the Co-authored-by trailer. I just did a test with a test SVN repository, and it looks like git-svn will insert a newline even if there are trailers at the end of the commit message. Perhaps we could fix "git-svn", but it doesn't look like we can add arbitrary git trailers today.

[normalperson (Eric Wong)]

[e-mail] wrote:

nobu (Nobuyoshi Nakada) wrote:

The feature page says "Include your trailers at the end of
your commit message", does it work even if "git-svn-id:"
line is appended by git svn dcommit?

It will work, but we have to ensure there is no newline
between the git-svn-id trailer and the Co-authored-by trailer.
I just did a test with a test SVN repository, and it looks
like git-svn will insert a newline even if there are trailers
at the end of the commit message. Perhaps we could fix
"git-svn", but it doesn't look like we can add arbitrary git
trailers today.

I wouldn't accept such a change to git-svn (explanation below).
Better to make trailer interpretation tolerate empty lines;
as trailers are a loose format, anyways (or filter out
git-svn-id: lines before interpreting trailers).

Since the earliest incarnations of git-svn in 2006, I've strived
to maintain deterministic behavior across git versions as long
as the same options (usually defaults) are used; so SHA-1
identifiers for commits could end up being identical (and thus
shareable) between contributors who previously had no contact.

[tenderlovemaking (Aaron Patterson)]

normalperson (Eric Wong) wrote:

[e-mail] wrote:

nobu (Nobuyoshi Nakada) wrote:

The feature page says "Include your trailers at the end of
your commit message", does it work even if "git-svn-id:"
line is appended by git svn dcommit?

It will work, but we have to ensure there is no newline
between the git-svn-id trailer and the Co-authored-by trailer.
I just did a test with a test SVN repository, and it looks
like git-svn will insert a newline even if there are trailers
at the end of the commit message. Perhaps we could fix
"git-svn", but it doesn't look like we can add arbitrary git
trailers today.

I wouldn't accept such a change to git-svn (explanation below).
Better to make trailer interpretation tolerate empty lines;
as trailers are a loose format, anyways (or filter out
git-svn-id: lines before interpreting trailers).

Since the earliest incarnations of git-svn in 2006, I've strived
to maintain deterministic behavior across git versions as long
as the same options (usually defaults) are used; so SHA-1
identifiers for commits could end up being identical (and thus
shareable) between contributors who previously had no contact.

Thanks for the background! Maybe we can get this fixed in git then. I'll see what I can find out!

[brianmario (Brian Lopez)]

normalperson (Eric Wong) wrote:

[e-mail] wrote:

nobu (Nobuyoshi Nakada) wrote:

The feature page says "Include your trailers at the end of
your commit message", does it work even if "git-svn-id:"
line is appended by git svn dcommit?

It will work, but we have to ensure there is no newline
between the git-svn-id trailer and the Co-authored-by trailer.
I just did a test with a test SVN repository, and it looks
like git-svn will insert a newline even if there are trailers
at the end of the commit message. Perhaps we could fix
"git-svn", but it doesn't look like we can add arbitrary git
trailers today.

I wouldn't accept such a change to git-svn (explanation below).
Better to make trailer interpretation tolerate empty lines;
as trailers are a loose format, anyways (or filter out
git-svn-id: lines before interpreting trailers).

Since the earliest incarnations of git-svn in 2006, I've strived
to maintain deterministic behavior across git versions as long
as the same options (usually defaults) are used; so SHA-1
identifiers for commits could end up being identical (and thus
shareable) between contributors who previously had no contact.

fwiw GitHub's implementation of trailer parsing is based on the rules defined by git-interpret-trailers. Which for parsing, are as follows:

Existing trailers are extracted from the input message by looking for a group of one or more lines that (i) are all trailers, or (ii) contains at least one Git-generated or user-configured trailer and consists of at least 25% trailers. The group must be preceded by one or more empty (or whitespace-only) lines. The group must either be at the end of the message or be the last non-whitespace lines before a line that starts with ---. Such three minus signs start the patch part of the message.

As such, as @tenderlove just pointed out, making it so the git-svn trailers are included in that list would require changes to git (and libgit2) and would break the existing trailer format. We can certainly propose a patch but I'm not confident it'll be accepted.

Better to make trailer interpretation tolerate empty lines;
as trailers are a loose format

It's true individual trailer lines are a lose format, but the definition for what a block of trailer(s) is, is not. That said, I fully understand the situation you're in. Changing the behavior in git-svn would come with its own trade-offs as well. Tricky...

[tenderlovemaking (Aaron Patterson)]

brianmario (Brian Lopez) wrote:

Better to make trailer interpretation tolerate empty lines;
as trailers are a loose format

It's true individual trailer lines are a lose format, but the definition for what a block of trailer(s) is, is not. That said, I fully understand the situation you're in. Changing the behavior in git-svn would come with its own trade-offs as well. Tricky...

I guess as a user I would expect git-svn to be additive; if there are no trailers in my commit message, then add them (for the id), otherwise add the id trailer to the trailers I specified.

[nobu (Nobuyoshi Nakada)]

brianmario (Brian Lopez) wrote:

fwiw GitHub's implementation of trailer parsing is based on the rules defined by git-interpret-trailers. Which for parsing, are as follows:

Thank you for the info.
git-interpret-trailers seems to accept git-svn trailer.

$ git cat-file commit @ | git interpret-trailers --parse
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@62413 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

But git-svn seems not using git-interpret-trailers, https://github.com/ruby/ruby/commit/6593c3af8d9490687180ae7f413561a41d5547d5

So what we can do is to send a patch to git-svn for git-interpret-trailers, I think.
We need Perl Mongers.

[greggzst (Grzegorz Jakubiak)]

+1 for this idea. I've got one pull request waiting to be merged and I'd love to be listed as a real contributor to ruby.

[hsbt (Hiroshi SHIBATA)]

• Status changed from Open to Closed

Today, We move to Git from Subversion.

https://www.ruby-lang.org/en/news/2019/04/23/move-to-git-from-svn/

We can keep your original commit now.