Skip to content

Conversation

@akashRindhe
Copy link
Contributor

#1099

Description

Replace complex parsing logic from GHEvent.type to GHEvent with static mapping

@codecov
Copy link

codecov bot commented Apr 21, 2021

Codecov Report

Merging #1126 (5c64fec) into main (f6e8a2c) will increase coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1126      +/-   ##
============================================
+ Coverage     73.14%   73.26%   +0.11%     
  Complexity     1824     1824              
============================================
  Files           185      185              
  Lines          6100     6116      +16     
  Branches        368      365       -3     
============================================
+ Hits           4462     4481      +19     
+ Misses         1417     1416       -1     
+ Partials        221      219       -2     
Impacted Files Coverage Δ Complexity Δ
src/main/java/org/kohsuke/github/GHEvent.java 100.00% <100.00%> (ø) 3.00 <0.00> (ø)
src/main/java/org/kohsuke/github/GHEventInfo.java 85.71% <100.00%> (+10.71%) 9.00 <1.00> (-1.00) ⬆️
...in/java/org/kohsuke/github/internal/EnumUtils.java 100.00% <100.00%> (ø) 4.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 f6e8a2c...5c64fec. 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.

This is not what I was expecting, but it works, is clearer, and it has to be faster than the old way.

Minor changes needed.

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.

If you use EnumUtils t hat would be better.

@akashRindhe
Copy link
Contributor Author

Thanks for making the changes. I believe everything is resolved now and is good to merge?

@bitwiseman bitwiseman merged commit 8868879 into hub4j:main Apr 22, 2021
@bitwiseman
Copy link
Member

@akashRindhe
FYI, I'm sure you could have made the changes yourself, but I felt like getting them done while I was looking at them. Hope you don't mind. Thanks for your contributions.

@akashRindhe
Copy link
Contributor Author

@akashRindhe
FYI, I'm sure you could have made the changes yourself, but I felt like getting them done while I was looking at them. Hope you don't mind. Thanks for your contributions.

No issues mate. Cheers :)

@akashRindhe akashRindhe deleted the refactor/1099 branch May 9, 2021 02:25
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.

2 participants