Skip to content

Conversation

@sliverc
Copy link
Member

@sliverc sliverc commented Apr 16, 2018

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

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
@codecov-io
Copy link

Codecov Report

Merging #419 into master will increase coverage by 0.08%.
The diff coverage is 96.42%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
example/tests/test_model_viewsets.py 100% <100%> (ø) ⬆️
rest_framework_json_api/settings.py 100% <100%> (ø)
example/tests/unit/test_settings.py 100% <100%> (ø)
rest_framework_json_api/parsers.py 97.14% <100%> (ø) ⬆️
rest_framework_json_api/exceptions.py 84.61% <100%> (ø) ⬆️
example/tests/test_generic_viewset.py 100% <100%> (ø) ⬆️
example/tests/unit/test_utils.py 100% <100%> (ø) ⬆️
rest_framework_json_api/utils.py 90.75% <71.42%> (+0.03%) ⬆️

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 e501b04...7252be3. Read the comment docs.

@sliverc sliverc mentioned this pull request Apr 16, 2018
5 tasks
@sliverc sliverc requested a review from n2ygk May 1, 2018 08:39
@sliverc
Copy link
Member Author

sliverc commented May 1, 2018

@n2ygk Could you have a look at this? Thanks.

Copy link
Contributor

@n2ygk n2ygk left a 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,
    ...

@sliverc
Copy link
Member Author

sliverc commented May 1, 2018

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... ;).

@sliverc sliverc merged commit acca7c1 into django-json-api:master May 1, 2018
@sliverc sliverc deleted the rework_settings branch December 28, 2021 18:52
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