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