Proposed revised format for JDK commit messages

[Erik Helin]

On 09/19/2018 10:30 AM, Alan Bateman wrote:
> On 19/09/2018 08:20, Per Liden wrote:
>>
>> It would feel natural to have all *-by lines follow the same syntax. 
>> The Reviewed-by line sticks out in this regard. Is there a reason why 
>> we don't just do:
>>
>> Reviewed-by: Some Name <some.name at oracle.com>
>> Reviewed-by: Some Other Name <some.other.name at oracle.com>
>>
>> ... and so on?
> I assume the names specified to the Reviewed-by line(s) will need to be 
> checked against the set of members of the project and their roles. I 
> don't know the details of the validation that is done today but it might 
> be less reliable to try to map anything other than their OpenJDK user name.

I don't have any strong opinion on whether to go with with one 
"Reviewed-by" line or multiple, I just want to point out that we _can_ 
go with multiple "Reviewed-by" lines if we want to. We can utilize what 
we already have done for the author field:

   "Real Name <username at openjdk.java.net>"

For an example have a look at [0], which has
"Per Liden <pliden at openjdk.java.net>" as the author. This way you know 
that this commit was done by an OpenJDK author and you also get their 
real name (please note that GitHub will not show the email address). 
Personally I also think it reads nicely, the commit was done by "Per 
Liden" who is also known as "pliden" on openjdk.java.net.

The motivation for going with one line was because it is more concise 
and because reviewers must always be OpenJDK authors (every reviewer 
must have an OpenJDK username). For shorter commit messages, with a 
single author and one to two reviewers, this becomes nice and succinct:

     8210283: Support git as an SCM alternative in the build

     Reviewed-by: ihse, ehelin

On the other hand I can see where Per is coming from for larger commits 
with multiple authors and potentially many reviewers. For those commits, 
having multiple "Reviewed-by" lines fit in well:

     8167108: inconsistent handling of SR_lock can lead to crashes

     Add Thread Safe Memory Reclamation (Thread-SMR) mechanism.

     Reviewed-by: Coleen Phillimore <coleenp at openjdk.java.net>
     Reviewed-by: Daniel Daugherty <dcubed at openjdk.java.net>
     Reviewed-by: David Holmes <dholmes at openjdk.java.net>
     Reviewed-by: Erik Osterlund <eosterlund at openjdk.java.net>
     Reviewed-by: Gerald Thornbrugh <gthornbr at openjdk.java.net>
     Reviewed-by: Kim Barrett <kbarrett at openjdk.java.net>
     Reviewed-by: Robbin Ehn <rehn at openjdk.java.net>
     Reviewed-by: Serguei Spitsyn <sspitsyn at openjdk.java.net>
     Reviewed-by: Stefan Karlsson <stefank at openjdk.java.net>
     Co-authored-by: Erik Osterlund <erik.osterlund at oracle.com>
     Co-authored-by: Robbin Ehn <robbin.ehn at oracle.com>
     Co-authored-by: Igor Ignatyev <igor.ignatyev at oracle.com>

With these many reviewers, another benefit is that each individual line 
becomes shorter (even though there are more of them).

For making a decision here, it might be worthwhile to keep in mind that 
the vast majority of commits in jdk/jdk have a single author and one to 
two reviewers, that is by far the most common case.

Again, I don't have strong preference here, I just want to highlight 
what can be done.

Thanks,
Erik

[0]: 
https://github.com/openjdk/jdk/commit/300babb5d7d28463e79c18b63f182149e66eca65