github-api: Convert methods with checked exceptions to unchecked (support streaming and functional programming)

Hi, GHObject#getCreatedAt and GHObject#getUpdatedAt throw unnecessary IOException which is not being thrown by an underlying code. Is there a reason for this?

About this issue

  • Original URL
  • State: open
  • Created a year ago
  • Comments: 19 (10 by maintainers)

Most upvoted comments

hi @bitwiseman, sorry for re-iterating this, am I right to think that this

If you’d like to start work on a v2.x of this library, I’d be happy to talk about features and direction.

means that you don’t see the possibility of publishing a new major version with breaking changes without introducing new major features at the same time viable?

I’m asking this because I’m reaching the point of considering forking the repo with private patches rather than maintaining numerous workarounds which I have many (#1739, #1689, #1656, you name it).

Getting rid of checked exceptions sounds like a thing I might be able to contribute, it’s the changes I’m personally interested in in the end, but the scope you suggested exceeds far beyond my availability. Unfortunately, I can’t offer my contribution in the form of shaping a whole new version of the library with big features I have no interest in for the sake of seeing the things I need to go live alongside them.

Forking = merge-hell

Do you really mean getting rid of all checked exceptions on all classes? If so, you’re talking about a pretty big change that will have ripple effects inside this library so it can’t just be done automatically via regex or some tooling. If you are talking about only removing some or most checked exceptions, you’re still talking about something that can’t be done automatically.

I would strongly suggest you not fork to achieve your goal here. Merging new changes from upstream given the above scenario will be an absolute nightmare.

Non-breaking AND stream-friendly

Let’s talk for a minute about one way this could be done without breaking changes and without forking to a separate repo. We could create new classes that extend or wrap the current classes and implement methods that do not throw checked exceptions. These could either go be a nested Unchecked class inside the current classes. The new classes would look something like this:

public class GHPullRequest extends GHIssue implements Refreshable {

  // { ... The current GHPullRequest class code ... } 

    public Unchecked toUnchecked() {
        return new Unchecked();
    }
        
    /**
     * GHPullRequest with no checked exception methods.
     */
    public class Unchecked {

        public boolean isDraft() {
            try {
                return GHPullRequest.this.isDraft();
            } catch (IOException e) {
                throw sneakyThrow(e);
            }
        } 

        public boolean isMerged() {
            try {
                return GHPullRequest.this.isMerged();
            } catch (IOException e) {
                throw sneakyThrow(e);
            }
        } 

        // Using lombok the method above would be even simpler
        // https://projectlombok.org/features/SneakyThrows
        //
        // @SneakyThrows
        // public boolean isMerged() {
        //     return GHPullRequest.this.isMerged();
        // } 

    // ... More unchecked methods as needed. 

    }
}

It’s a bunch of new code, but it is “pay-as-you-go” and the code itself is extremely simple. As you encounter methods that are throwing exceptions that you think should not, you can create the new class and add methods. On the regular classes we add a toUnchecked() method that returns the unchecked wrapper.

This solution could result in the above code looking like this:

            .stream()
            // Convert to unchecked wrapper
            .map(pr -> pr.toUnchecked())
            .filter(pr -> !pr.isMerged())
            .filter(pr -> !pr.isDraft())

I’m not loving the term “unchecked”. Maybe “streamable” or “functional”? Other that that, I think this could work really well.

What do you think?

New major version would need to be more, but not a lot more.

As to a new major version, I haven’t thought to deeply about it. That’s why I said “talk about”. I’m not set on introducing new major features, but if we are going to do a breaking change, we should at least look at other cleanup tasks to include in there.

Other changes to consider:

  • Remove all code marked deprecated.
  • Remove all current bridge methods.

There’s a couple other API restructuring tasks that I’d want to try to include as well.
But all this would still need to be non-breaking to the current API. I’d want to basically start a new package, maybe call it org.kohsuke.github-api.v2 and add a parallel implementation.

@bitwiseman It prevents end users from using library’s calls within Java streams.