Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SCHEDULE to GHEvent and add UNKNOWN handling to GHEvent #1096

Merged
merged 4 commits into from
Apr 14, 2021

Conversation

gsmet
Copy link
Contributor

@gsmet gsmet commented Apr 14, 2021

SCHEDULE is a valid event for scheduled workflow runs.

Also make GHEvent parsing more permissive as per our discussion from the previous weeks.

This is a valid event for scheduled workflow runs.
@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #1096 (6efe428) into master (e66a723) will increase coverage by 0.02%.
The diff coverage is 85.71%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1096      +/-   ##
============================================
+ Coverage     72.80%   72.82%   +0.02%     
- Complexity     1813     1814       +1     
============================================
  Files           185      185              
  Lines          6078     6083       +5     
  Branches        366      366              
============================================
+ Hits           4425     4430       +5     
  Misses         1432     1432              
  Partials        221      221              
Impacted Files Coverage Δ Complexity Δ
src/main/java/org/kohsuke/github/GHEventInfo.java 75.00% <0.00%> (ø) 10.00 <0.00> (ø)
src/main/java/org/kohsuke/github/GHEvent.java 100.00% <100.00%> (ø) 3.00 <0.00> (ø)
...rc/main/java/org/kohsuke/github/GHWorkflowRun.java 83.09% <100.00%> (ø) 34.00 <1.00> (ø)
...in/java/org/kohsuke/github/internal/EnumUtils.java 100.00% <100.00%> (ø) 3.00 <1.00> (+1.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e66a723...6efe428. Read the comment docs.

Copy link
Member

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!
Could you also update GHEventInfo#getType()?

Can be useful to catch values we should add instead of simply ignoring
them.
@gsmet
Copy link
Contributor Author

gsmet commented Apr 14, 2021

@bitwiseman I added two commits:

  • the first one returns UNKNOWN in GHEventInfo.getType but I haven't changed the logic as it's doing some manipulation we don't do in EnumUtils (and they look a bit too aggressive to be a good fit for a general purpose util)
  • I added a log warning if we don't find a value. This way, we can get some feedback about the values that should be added in GitHub API.

Comment on lines 53 to 55
if (e.name().replace("_", "").equalsIgnoreCase(t))
return e;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, I see the problem now. All this is to handle the completely different strings used in the REST API and actual event calls.
https://github.com/hub4j/github-api/blob/master/src/test/resources/org/kohsuke/github/AppTest/wiremock/testEventApi/__files/events-10.json#L283

There's got to be a better way to do this. I'll file an issue to come back to this.

@bitwiseman bitwiseman merged commit 8842489 into hub4j:master Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants