RFR: Issue tracker support

Robin Westberg rwestberg at openjdk.org
Mon Sep 30 14:46:30 UTC 2019


On Mon, 30 Sep 2019 13:40:41 GMT, Erik Helin <ehelin at openjdk.org> wrote:

> On Wed, 25 Sep 2019 13:14:38 GMT, Robin Westberg <rwestberg at openjdk.org> wrote:
> 
>> Hi all,
>> 
>> Please review this change that introduces issue support. An Issue contains a subset of PullRequest functionality, similar to how this is modeled in GitHub and GitLab. It also allows creating support for other issue trackers that do not have repository support, such as Jira.
>> 
>> Best regards,
>> Robin
>> 
>> ----------------
>> 
>> Commits:
>>  - 7b0e6107: Implement test version of Issue and IssueProject
>>  - b603a667: GitHub and GitLab does not yet implement Issues
>>  - d1886dcc: Extract IssueProject and Issue from HostedRepository and PullRequest
>> 
>> Changes: https://git.openjdk.java.net/skara/pull/162/files
>>  Webrev: https://webrevs.openjdk.java.net/skara/162/webrev.00
>>   Stats: 854 lines in 20 files changed: 608 ins; 240 del; 6 mod
>>   Patch: https://git.openjdk.java.net/skara/pull/162.diff
>>   Fetch: git fetch https://git.openjdk.java.net/skara pull/162/head:pull/162
> 
> _Very_ nice work Robin to have `PullRequest implements Issue` and `HostedRepository implements IssueTracker`. I see that you basically copied parts of the `PullRequest` and `HostedRepository` interfaces,  I would prefer to take this opportunity to update some of the naming conventions for methods (in particular dropping the "get" prefix). I also think that that the `Issue` and `IssueTracker` interfaces should be moved to a separate module: `issuetracker`. Now sure how much of this you want to do in this PR or if you want to integrate this first and the adapt the code?
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 40:
> 
>> 39:      */
>> 40:     String getId();
>> 41: 
> 
> Please drop the "get" and just use `id`
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 46:
> 
>> 45:      */
>> 46:     HostUserDetails getAuthor();
>> 47: 
> 
> Again, drop the "get"
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 52:
> 
>> 51:      */
>> 52:     String getTitle();
>> 53: 
> 
> ...and drop the "get" ��
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 64:
> 
>> 63:      */
>> 64:     String getBody();
>> 65: 
> 
> Drop "get" here as well
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 76:
> 
>> 75:      */
>> 76:     List<Comment> getComments();
>> 77: 
> 
> and drop "get" here too
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 95:
> 
>> 94:      */
>> 95:     ZonedDateTime getCreated();
>> 96: 
> 
> I would suggest renaming `getCreated()` to `createdAt()`
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 101:
> 
>> 100:      */
>> 101:     ZonedDateTime getUpdated();
>> 102: 
> 
> I would suggest renaming `getUpdated` to `updatedAt`
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 130:
> 
>> 129:      */
>> 130:     List<String> getLabels();
>> 131: 
> 
> Drop the "get" here too
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 135:
> 
>> 134:      */
>> 135:     URI getWebUrl();
>> 136: 
> 
> Drop "get" here as well
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 140:
> 
>> 139:      */
>> 140:     List<HostUserDetails> getAssignees();
>> 141: 
> 
> Drop "get" here too
> 
> host/src/main/java/org/openjdk/skara/host/Issue.java line 146:
> 
>> 145:      */
>> 146:     void setAssignees(List<HostUserDetails> assignees);
>> 147: }
> 
> Rename this from `setAssignees` to `assignTo`?

Right, as the PullRequest interface now extends the Issue interface, renaming these methods would touch pretty much every file in the bots directory I guess.. ��  But yes, would be nice to clean up.

(While I'm at it, I'll have to check if the mailing list archive correctly displays these: åäö and é) :)

PR: https://git.openjdk.java.net/skara/pull/162


More information about the skara-dev mailing list