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

Split requirements files up following DRF example #720

Merged
merged 1 commit into from
Oct 10, 2019

Conversation

sliverc
Copy link
Member

@sliverc sliverc commented Oct 9, 2019

Fixes #712
Fixes #635

Description of the Change

This way not all dependencies need to be installed for the different testing steps.

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

@sliverc sliverc force-pushed the requirements_files branch from 890870b to 63b1834 Compare October 9, 2019 07:35
@codecov
Copy link

codecov bot commented Oct 9, 2019

Codecov Report

Merging #720 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #720   +/-   ##
=======================================
  Coverage   97.08%   97.08%           
=======================================
  Files          54       54           
  Lines        2742     2742           
=======================================
  Hits         2662     2662           
  Misses         80       80

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 7655f02...ed10e03. Read the comment docs.

@sliverc sliverc marked this pull request as ready for review October 9, 2019 07:38
@sliverc sliverc requested a review from n2ygk October 9, 2019 07:38
@sliverc
Copy link
Member Author

sliverc commented Oct 9, 2019

@n2ygk
This PR is just a start but requirements can been cleaned up even more when following issue are worked on.

#721
#468

However this is a start. Once this is merged I would like to release DJA 3.0 what do you think?

@sliverc sliverc force-pushed the requirements_files branch from 63b1834 to aa8c06a Compare October 9, 2019 07:44
@n2ygk
Copy link
Contributor

n2ygk commented Oct 10, 2019

@sliverc Can you figure out why the sphinx build is failing? I've been staring at it and don't see why it shoud.

@sliverc
Copy link
Member Author

sliverc commented Oct 10, 2019

@n2ygk
I don't get it either. Although when I delete the .tox/docs and docs/apidoc folder locally I can reproduce the error when running tox -e docs, I don't quite understand though why.

@sliverc sliverc force-pushed the requirements_files branch from fe8cf34 to 911a3b9 Compare October 10, 2019 16:52
Fixes django-json-api#712
Fixes django-json-api#635

This way not all dependency need to be installed for the different
testing steps.
@sliverc sliverc force-pushed the requirements_files branch from 911a3b9 to ed10e03 Compare October 10, 2019 16:54
@sliverc
Copy link
Member Author

sliverc commented Oct 10, 2019

@n2ygk I think I have found it. Seems to be a regression of #711 Hope CI will pass now.

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.

LGTM

@n2ygk n2ygk merged commit dca25e1 into django-json-api:master Oct 10, 2019
sliverc added a commit to sliverc/django-rest-framework-json-api that referenced this pull request Oct 3, 2021
Fixes django-json-api#518
Fixes django-json-api#720

This is to avoid an incomprehensible exception during runtime
when either meta or results is used as a field name.
@sliverc sliverc deleted the requirements_files branch December 28, 2021 18:53
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.

Test sphinx docs build in test Tox using pytest-runner setup scripts leads to invalid deps
2 participants