RFR: 1997: Allow the creator of a PR to set the commit author [v3]

Erik Joelsson erikj at openjdk.org
Fri Sep 1 20:34:39 UTC 2023


On Fri, 1 Sep 2023 20:29:24 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> In this pr, we will introduce a new pull request command `/author`.
>> 
>> If an author wants to contribute a change to a repository, but he doesn't have access, then someone with the access will be able to help him with this command.
>> 
>> Usage: `/author [set|remove] [@user | openjdk-user | Full Name <email at address>]`
>> Examples: 
>> `/author set @openjdk-bot`
>> `/author @openjdk-bot`
>> `/author set J. Duke <duke at openjdk.org>`
>> `/author remove @openjdk-bot`
>> `/author remove`
>> 
>> Note: only Committers of the repository would be allowed to issue /author command.
>
> Zhao Song has updated the pull request incrementally with 10 additional commits since the last revision:
> 
>  - fix tests
>  - update
>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>  - update
>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>

Reading back my language suggestions again now, I found some more fine tuning could be done to make it read even better and clearer.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java line 108:

> 106:                 }
> 107:                 if (currAuthor.isEmpty()) {
> 108:                     reply.println("There is no overriding author set in this pull request.");

I think "for" reads slightly better.
Suggestion:

                    reply.println("There is no overriding author set for this pull request.");

bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java line 112:

> 110:                     if (currAuthor.get().equals(author.get())) {
> 111:                         reply.println(OverridingAuthor.removeAuthorMarker(author.get()));
> 112:                         reply.println("Overriding author `" + author.get() + "` was successfully removed. When this pull request is integrated, the pull request author will be the author of the commit.");

Reading this again, it can be misinterpreted.
Suggestion:

                        reply.println("Overriding author `" + author.get() + "` was successfully removed. When this pull request is integrated, the pull request author will be used as the author of the commit.");

bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java line 124:

> 122:     @Override
> 123:     public String description() {
> 124:         return "sets an overriding author used in the final commit when the PR is integrated";

Suggestion:

        return "sets an overriding author to be used in the commit when the PR is integrated";

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java line 159:

> 157:     }
> 158: 
> 159:     Hash commit(Hash finalHead, Namespace namespace, String censusDomain, String sponsorId, Hash original, List<Comment> comments) throws IOException, CommitFailure {

Can you also replace the usage in `commitMessage` in this class.

-------------

PR Review: https://git.openjdk.org/skara/pull/1553#pullrequestreview-1607355482
PR Review Comment: https://git.openjdk.org/skara/pull/1553#discussion_r1313294667
PR Review Comment: https://git.openjdk.org/skara/pull/1553#discussion_r1313296087
PR Review Comment: https://git.openjdk.org/skara/pull/1553#discussion_r1313297329
PR Review Comment: https://git.openjdk.org/skara/pull/1553#discussion_r1313298631


More information about the skara-dev mailing list