-
Notifications
You must be signed in to change notification settings - Fork 299
Add json api settings module #419
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
Conversation
This is inspired by api_settings of REST framework. Advantages: * Centralizing all json api settings and defaults in one module * Simplifying refactoring and deprecation of settings in the future * Properly use override_settings in tests instead of monkey patching
da0ef63 to
7252be3
Compare
codecov-io
commented
Apr 16, 2018
Codecov Report
@@ Coverage Diff @@ ## master #419 +/- ## ========================================== + Coverage 91.75% 91.83% +0.08% ========================================== Files 55 57 +2 Lines 2923 2953 +30 ========================================== + Hits 2682 2712 +30 Misses 241 241
Continue to review full report at Codecov.
|
@n2ygk Could you have a look at this? Thanks.
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.
This looks good as an intermediate step to perhaps a future (breaking) version in which these settings are in a dict instead of having a common prefix (as is done in DRF):
JSON_API_FORMAT_KEYS = 'camelize'
JSON_API_FORMAT_TYPES = 'camelize'
REST_FRAMEWORK = {
'PAGE_SIZE': 5,
...
Thanks for review. Merging it therefore. Yep, we could certainly think about moving to a dict approach as DRF does. One step at the time... ;).
This is inspired by api_settings of REST framework.
Advantages: