-
Notifications
You must be signed in to change notification settings - Fork 202
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
Allow users to define its own set of Cacheable Status Code when using Faraday Middleware Cache #275
Conversation
… be within CACHEABLE_STATUS_CODE pre-defined constant values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Physium thank you so much for submitting this PR 🙌 !
The overall implementation works and tests are already passing, but I highlighted a few things I'd like to change before merging.
I hope this will be a good opportunity to learn a few things.
Please also remember to add the new option to the initialize
method documentation as well 🙏 !
1. memozied method to prevent unnecessary calls when called multiple times 2. shift logic into custom_status_codes 3. rename custom_status_code to custom_status_codes
Thank you @olleolleolle, that's a great suggestion! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Physium It looks great 💪 , thanks for incorporating all our comments 🙌!
LGTM 🎉 !
This implementation allows users to define its own set of status codes for caching as opposed to sticking to a set of predefined CACHEABLE_STATUS_CODE constant. Nonetheless, user defined status code has to be within the list of predefined CACHEABLE_STATUS_CODE constant.