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

refactor(core-api): integrate hapi-pagination to replace fork #2994

Merged
merged 7 commits into from
Oct 2, 2019

Conversation

faustbrian
Copy link
Contributor

@faustbrian faustbrian commented Sep 29, 2019

Summary

Integrate the https://github.com/faustbrian/hapi-pagination repository directly into core-api and update all hapi dependencies to the @hapi/* namespace.

This gets rid of a bunch of dependencies and the need to keep a fork alive, also gives us the freedom to make any change we need to do. Also removes the warning about the outdated hapi scope when you install core.

  • Removes a lot of configuration values. Only the page limit value can be configured.
  • Removes a lot of logic that revolved around the configuration values which have been removed. This removes a little bit of overhead from each request.
  • Route registrations are now set inside the plugin with a method and path to avoid the hacky method check logic. This is more explicit and reliable, either a route matches or it doesn't.

Checklist

  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@codecov
Copy link

codecov bot commented Sep 29, 2019

Codecov Report

Merging #2994 into develop will decrease coverage by <.01%.
The diff coverage is 68.75%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2994      +/-   ##
===========================================
- Coverage    66.08%   66.07%   -0.01%     
===========================================
  Files          420      423       +3     
  Lines        10046    10100      +54     
  Branches       514      532      +18     
===========================================
+ Hits          6639     6674      +35     
- Misses        3364     3382      +18     
- Partials        43       44       +1
Impacted Files Coverage Δ
...kages/core-api/src/handlers/transactions/routes.ts 100% <ø> (ø) ⬆️
packages/core-api/src/server.ts 65.78% <100%> (+0.92%) ⬆️
packages/core-api/src/plugins/pagination/config.ts 100% <100%> (ø)
...ckages/core-api/src/plugins/pagination/decorate.ts 20% <20%> (ø)
packages/core-api/src/plugins/pagination/ext.ts 81.81% <81.81%> (ø)
packages/crypto/src/transactions/verifier.ts 88% <0%> (-4%) ⬇️
packages/crypto/src/blocks/block.ts 87.87% <0%> (-1.34%) ⬇️
...se/src/repositories/wallets-business-repository.ts 58.57% <0%> (-0.85%) ⬇️
packages/crypto/src/validation/keywords.ts 91.52% <0%> (ø) ⬆️
...ackages/crypto/src/transactions/types/htlc-lock.ts 100% <0%> (ø) ⬆️
... and 11 more

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 ee6fe40...46fa96c. Read the comment docs.

@faustbrian faustbrian force-pushed the refactor/hapi-pagination branch 3 times, most recently from 59815da to 4f57459 Compare September 29, 2019 13:45
@spkjp spkjp merged commit 279585b into develop Oct 2, 2019
@ghost ghost deleted the refactor/hapi-pagination branch October 2, 2019 22:48
@dated dated mentioned this pull request Oct 2, 2019
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.

3 participants