Support state reason for issues #1793

stianst · 2024-02-14T07:38:11Z

Hi, first of thanks for providing this library. I'm using it quite a lot, but finding a few things that are missing for my needs, and trying to contribute those, with the first one being supporting closing issues as not planned.

Trying to figure out how to work with the testsuite, but having a few issues.

When running mvn install -Dtest.github.org=false -Dtest=GHIssueTest I'm getting the following exception:

Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field protected java.lang.String java.net.HttpURLConnection.method accessible: module java.base does not "opens java.net" to unnamed module @551bca18

Another thing is the test is hardwired to hub4j-test-org which I don't have access to, so guess I need to use my own repository somehow.

Closes #1792

gsmet · 2024-02-14T09:04:14Z

Hey @stianst ,

Yeah, you usually need access to the test org when recording testing as it's easier for them to be reproduced by others when something needs a change.

You need @bitwiseman for that.

As for the error you have when running the tests, the test runs by default with an HTTP Client that is not compatible with Java 17+. Run them with Java 11 and things should work better.

gsmet · 2024-02-14T09:14:05Z

Ah, actually, good news, I forgot I had the permission to do that.

I added you to the test org. You should have received an invite.

@bitwiseman Stian is a colleague from Red Hat leading the Keycloak project.

gsmet

I spotted a few minor issues.

gsmet · 2024-02-14T09:04:29Z

src/main/java/org/kohsuke/github/GHIssue.java

@@ -67,6 +69,9 @@ public class GHIssue extends GHObject implements Reactable {
    /** The state. */
    protected String state;

+    /** The state. */


This needs an update

gsmet · 2024-02-14T09:07:12Z

src/main/java/org/kohsuke/github/GHIssue.java

+     * @return the state reason
+     */
+    public GHIssueStateReason getStateReason() {
+        return Enum.valueOf(GHIssueStateReason.class, state_reason.toUpperCase(Locale.ENGLISH));


I know you were trying to be consistent with the one above but we have a new way of doing this:

add an UNKNOWN value to your enum

use EnumUtils.getEnumOrDefault() (there are plenty of examples in the source code). There is also a method supporting null value, you probably need to check if the state_reason can be null... which is probably the case. In this case, you would need to use getNullableEnumOrDefault.

Nice, that's much better. Updated to use that.

gsmet · 2024-02-14T09:08:42Z

src/main/java/org/kohsuke/github/GHIssueStateReason.java

+/*
+ * The MIT License
+ *
+ * Copyright (c) 2011, Eric Maupin


This is off. But I'm not sure if we are encouraged to put our name here, these days. @bitwiseman would know.

Looked at a couple recent commits and they don't have any license headers so removed this.

gsmet · 2024-02-14T09:09:16Z

src/main/java/org/kohsuke/github/GHIssueStateReason.java

+
+public enum GHIssueStateReason {
+
+    COMPLETED, NOT_PLANNED


As previously stated, you can add UNKNOWN here. This avoids errors when GitHub add a new value.

gsmet · 2024-02-14T09:28:19Z

Ah also, do not record things for all the tests, doing:

rm -rf src/test/resources/org/kohsuke/github/GHIssueTest/wiremock/closeIssue/
mvn install -Dtest.github.takeSnapshot -Dtest=GHIssueTest#closeIssue
rm -rf src/test/resources/org/kohsuke/github/GHIssueTest/wiremock/closeIssueNotPlanned/
mvn install -Dtest.github.takeSnapshot -Dtest=GHIssueTest#closeIssueNotPlanned

will be better.

Then you should add the new snapshots to your commit.

stianst · 2024-02-14T10:10:19Z

Got the test working eventually (well, okay I only spent 30 min, so this wasn't too bad ;)), but used an org I created for the purpose. Will try with the official org and update the data.

stianst · 2024-02-14T10:20:22Z

@gsmet @bitwiseman would it be okay to enable fine-grained personal access tokens for the org (https://github.com/organizations/hub4j-test-org/settings/personal-access-tokens-onboarding). They are a bit nicer than the legacy tokens as you can limit to a specific org/repo.

stianst · 2024-02-14T10:25:13Z

Should be ready to go now

gsmet

I added a very small comment for something that we need to check but otherwise it looks very good.

Note that we will have to wait for @bitwiseman to merge it as I'm not a committer here.

gsmet · 2024-02-14T15:36:17Z

src/main/java/org/kohsuke/github/GHIssue.java

+     * @return the state reason
+     */
+    public GHIssueStateReason getStateReason() {
+        return EnumUtils.getEnumOrDefault(GHIssueStateReason.class, state_reason, GHIssueStateReason.UNKNOWN);


Are you sure this can't be null? My understanding is that when the issue is not closed, it will be null but I might be mistaken.
If so, please use EnumUtils.getNullableEnumOrDefault(). I pointed you to it in my previous comment but it might have been a bit confusing as I pointed to this one first.

Updated, I was kinda thinking it should be set to UNKNOWN when null, but obviously null makes more sense ;)

bitwiseman · 2024-02-15T22:35:34Z

@gsmet @bitwiseman would it be okay to enable fine-grained personal access tokens for the org (https://github.com/organizations/hub4j-test-org/settings/personal-access-tokens-onboarding). They are a bit nicer than the legacy tokens as you can limit to a specific org/repo.

Enabled the fine-grained PATs.

codecov · 2024-02-15T22:44:49Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (be00e51) 80.64% compared to head (ddaec5f) 80.69%.
Report is 4 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1793      +/-   ##
============================================
+ Coverage     80.64%   80.69%   +0.04%     
- Complexity     2322     2328       +6     
============================================
  Files           219      220       +1     
  Lines          7027     7043      +16     
  Branches        371      371              
============================================
+ Hits           5667     5683      +16     
  Misses         1128     1128              
  Partials        232      232

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Closes hub4j#1792

stianst · 2024-02-16T07:25:02Z

Fixed the last issue with using EnumUtils.getNullableEnumOrDefault and added Javadocs to GHIssueStateReason since the site job complained about that.

stianst · 2024-02-20T16:14:40Z

@bitwiseman hi, sorry to ping you on this as I'm sure you're busy with other stuff. Just wondering a bit on when/if this could be merged and what the plans are for a new release? I'm a bit blocked on this for some automation for the Keycloak project.

src/main/java/org/kohsuke/github/GHIssueStateReason.java

src/main/java/org/kohsuke/github/GHIssue.java

src/main/java/org/kohsuke/github/GHIssueStateReason.java

gsmet suggested changes Feb 14, 2024

View reviewed changes

stianst force-pushed the state_reason branch 2 times, most recently from a6ed0f6 to 0c41dd7 Compare February 14, 2024 09:37

stianst force-pushed the state_reason branch from 0c41dd7 to 4e24b7d Compare February 14, 2024 10:23

stianst marked this pull request as ready for review February 14, 2024 10:23

stianst force-pushed the state_reason branch 2 times, most recently from d911b1e to 5b3e6d0 Compare February 14, 2024 10:24

stianst requested a review from gsmet February 14, 2024 10:25

gsmet reviewed Feb 14, 2024

View reviewed changes

Support state reason for issues

599b78c

Closes hub4j#1792

stianst force-pushed the state_reason branch from 5b3e6d0 to 599b78c Compare February 16, 2024 07:23

gsmet approved these changes Feb 16, 2024

View reviewed changes

bitwiseman reviewed Feb 20, 2024

View reviewed changes

src/main/java/org/kohsuke/github/GHIssueStateReason.java Show resolved Hide resolved

bitwiseman reviewed Feb 20, 2024

View reviewed changes

src/main/java/org/kohsuke/github/GHIssue.java Outdated Show resolved Hide resolved

Apply suggestions from code review

c3e19c3

bitwiseman reviewed Feb 20, 2024

View reviewed changes

src/main/java/org/kohsuke/github/GHIssueStateReason.java Outdated Show resolved Hide resolved

Update src/main/java/org/kohsuke/github/GHIssueStateReason.java

ddaec5f

bitwiseman approved these changes Feb 20, 2024

View reviewed changes

bitwiseman merged commit 5c155dc into hub4j:main Feb 20, 2024
11 checks passed

stianst deleted the state_reason branch February 22, 2024 13:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support state reason for issues #1793

Support state reason for issues #1793

stianst commented Feb 14, 2024 •

edited

Loading

gsmet commented Feb 14, 2024

gsmet commented Feb 14, 2024

gsmet left a comment

gsmet Feb 14, 2024

gsmet Feb 14, 2024

stianst Feb 14, 2024

gsmet Feb 14, 2024

stianst Feb 14, 2024

gsmet Feb 14, 2024

gsmet commented Feb 14, 2024

stianst commented Feb 14, 2024

stianst commented Feb 14, 2024

stianst commented Feb 14, 2024

gsmet left a comment

gsmet Feb 14, 2024

stianst Feb 16, 2024

bitwiseman commented Feb 15, 2024

codecov bot commented Feb 15, 2024 •

edited

Loading

stianst commented Feb 16, 2024

stianst commented Feb 20, 2024

Support state reason for issues #1793

Support state reason for issues #1793

Conversation

stianst commented Feb 14, 2024 • edited Loading

gsmet commented Feb 14, 2024

gsmet commented Feb 14, 2024

gsmet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsmet commented Feb 14, 2024

stianst commented Feb 14, 2024

stianst commented Feb 14, 2024

stianst commented Feb 14, 2024

gsmet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bitwiseman commented Feb 15, 2024

codecov bot commented Feb 15, 2024 • edited Loading

Codecov Report

stianst commented Feb 16, 2024

stianst commented Feb 20, 2024

stianst commented Feb 14, 2024 •

edited

Loading

codecov bot commented Feb 15, 2024 •

edited

Loading