Skip to content

Conversation

dziemba
Copy link
Member

@dziemba dziemba commented Apr 13, 2022

In #82 we
added a custom deserializer for the Branch class.

This deserializer didn't handle certain cases properly that the GH API
would return. In particular the case where the protected field was set
and no protection_url is present would lead to a NPE in the
deserializer.

To avoid this error and make the whole parser more resilient to
different API results, let's reduce the scope of the custom deserializer
to only parse the protection_url field instead of the whole Branch
class.

There should be no change in runtime behavior apart from fixing the
error cases.

In #82 we
added a custom deserializer for the `Branch` class.

This deserializer didn't handle certain cases properly that the GH API
would return. In particular the case where the `protected` field was set
and no `protection_url` is present would lead to a NPE in the
deserializer.

To avoid this error and make the whole parser more resilient to
different API results, let's reduce the scope of the custom deserializer
to only parse the `protection_url` field instead of the whole `Branch`
class.

There should be no change in runtime behavior apart from fixing the
error cases.
@codecov
Copy link

codecov bot commented Apr 13, 2022

Codecov Report

Merging #89 (547597f) into master (c029a8b) will decrease coverage by 0.41%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master      #89      +/-   ##
============================================
- Coverage     70.20%   69.78%   -0.42%     
  Complexity      222      222              
============================================
  Files            36       36              
  Lines           866      854      -12     
  Branches         36       35       -1     
============================================
- Hits            608      596      -12     
  Misses          233      233              
  Partials         25       25              
Impacted Files Coverage Δ
...thub/v3/repos/BranchProtectionUrlDeserializer.java 100.00% <100.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 1155a5b...547597f. Read the comment docs.

Copy link
Contributor

@sathish-kumar-subramani sathish-kumar-subramani left a comment

Choose a reason for hiding this comment

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

LGTM

@dziemba dziemba merged commit 7b60781 into master Apr 15, 2022
@dziemba dziemba deleted the fix-branch-protection-url-deserialize branch April 15, 2022 13:01
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