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

wrapper functions for endpoint calls #73

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

bocops
Copy link
Collaborator

@bocops bocops commented Jan 3, 2023

Replace direct use of DELETE/GET/PATCH/POST functions with getMastodonRequest() and related functions where possible, reducing exposure of Gson serializer.

Some special cases remain:

  • Media.kt
  • Streaming.kt
  • postUserNameAndPassword() in sample apps

After cleanup of those and related tests, Gson instance no longer needs to be exposed via MastodonClient.getSerializer, and #51 can be closed.

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Merging #73 (d41c652) into master (104f880) will decrease coverage by 3.25%.
The diff coverage is 57.34%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #73      +/-   ##
============================================
- Coverage     44.68%   41.42%   -3.26%     
+ Complexity      169      167       -2     
============================================
  Files            62       62              
  Lines          1410     1376      -34     
  Branches         82       82              
============================================
- Hits            630      570      -60     
- Misses          762      785      +23     
- Partials         18       21       +3     
Impacted Files Coverage Δ
.../kotlin/social/bigbone/api/method/MastodonLists.kt 0.00% <0.00%> (ø)
...e/src/main/kotlin/social/bigbone/MastodonClient.kt 10.13% <9.78%> (+2.44%) ⬆️
...kotlin/social/bigbone/api/method/FollowRequests.kt 42.85% <40.00%> (ø)
.../kotlin/social/bigbone/api/method/Notifications.kt 58.82% <53.84%> (-6.18%) ⬇️
...rc/main/kotlin/social/bigbone/api/method/Public.kt 80.64% <81.81%> (-3.57%) ⬇️
.../main/kotlin/social/bigbone/api/method/Statuses.kt 85.71% <85.71%> (-0.40%) ⬇️
.../main/kotlin/social/bigbone/api/method/Accounts.kt 88.23% <87.71%> (-0.99%) ⬇️
.../src/main/kotlin/social/bigbone/api/method/Apps.kt 92.50% <100.00%> (-0.19%) ⬇️
...rc/main/kotlin/social/bigbone/api/method/Blocks.kt 100.00% <100.00%> (ø)
...ain/kotlin/social/bigbone/api/method/Favourites.kt 100.00% <100.00%> (ø)
... and 10 more

Replace direct use of DELETE/GET/PATCH/POST functions with getMastodonRequest() and related functions where possible, reducing exposure of Gson serializer.

Some special cases remain:
- Media.kt
- Streaming.kt
- postUserNameAndPassword() in sample apps

After cleanup of those and related tests, Gson instance no longer needs to be exposed via MastodonClient.getSerializer, and andregasser#51 can be closed.
@andregasser
Copy link
Owner

andregasser commented Jan 4, 2023

Sorry, had to tune CodeCov first, as their annotations polluted the diffs way too much. I am still not 100% happy with the CodeCov tool yet, but its less annoying than before. I still do not get how they calculate their metrics. But maybe it is because it is not configured correctly yet. Found some more info about it. Maybe will take another approach fixing it.

Copy link
Owner

@andregasser andregasser left a comment

Choose a reason for hiding this comment

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

That's great work! Highly appreciated. Looks much cleaner now. Just realized I need to dive into reified inline functions a bit more :-)

@bocops bocops merged commit 385ecd5 into andregasser:master Jan 4, 2023
@bocops bocops deleted the 51_reduce_gson_exposure branch January 4, 2023 11:08
@bocops
Copy link
Collaborator Author

bocops commented Jan 4, 2023

Yeah. The newly introduced functions in MastodonClient.kt are not being tested - but they are also just inlined wrappers around the functions that were previously called directly, so I think most of that "reduced coverage" is just an artifact of that.

I have some doubts about the testing that is currently going on, anyway, so increasing test coverage while we deal with individual entities and methods seems to be the best approach here.

Thanks for the review, merged and done! :)

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