RFR: 1256: Provide a common method to get the corresponding csr issue

[Erik Joelsson]

On Tue, 30 Nov 2021 13:26:12 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> Hi all,
> 
> This patch adds the common methods `Issue#linksWithRelationships`, `Issue#csrLink` and `Issue#csrIssue` and refactors the code to reduce the loop nested statements by using these common methods.
> 
> All the existing tests passed.
> 
> Best Regards,
> -- Guoxiong

I'm not sure about this change. The Issue interface seems over used and already contains a bunch of methods that do not apply to several implementors. I don't like the idea of adding even more such methods.

I think we need to try to extract a sub interface from Issue, which is specialized for bugtrackers or Jira or something. Not sure what to name it at this point though. All the links  methods should be moved there, and methods that definitely can't return a pull request, like IssueProject::issue should return the new interface instead. This is part of what I meant with this being hard to solve properly.

issuetracker/src/main/java/org/openjdk/skara/issuetracker/Issue.java line 196:

> 194:      * @return the related links
> 195:      */
> 196:     default List<Link> linksWithRelationships(List<String> relationships) {

I'm not sure there will ever be a need to filter links with multiple different relationships. Do you know of any such case that you are planning to convert?

The rest of this method could be converted to streams like this:


return links().stream()
    .filter(l -> link.relationship().isPresent() && relationships.contains(link.relationship().get())
    .toList();

issuetracker/src/main/java/org/openjdk/skara/issuetracker/Issue.java line 215:

> 213:     default Optional<Link> csrLink() {
> 214:         var links = linksWithRelationships(List.of("csr for"));
> 215:         if (links != null && !links.isEmpty()) {

>From what I can tell, `links` can never be null here. 

I think this method could be expressed a bit more elegantly with stream usage, like this:

return links.stream().findAny();

issuetracker/src/main/java/org/openjdk/skara/issuetracker/Issue.java line 228:

> 226:     default Optional<Issue> csrIssue() {
> 227:         var csrLink = csrLink();
> 228:         var csrIssue = Optional.<Issue>empty();

This could be simplified to:


return csrLink().map(Link::issue);

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

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