-
Notifications
You must be signed in to change notification settings - Fork 299
Issue 410 #411
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
Issue 410 #411
Conversation
Resolves django-json-api#410: docs/getting-started.md example doesn't work.
Ugh, looks like my edit to example/urls.py messed up.
...orted properly. - travis caught it. This time I've done the isort locally to confirm the order. - MIDDLEWARE_CLASSES was replaced by MIDDLEWARE as of Django 1.10 and, according to tox.ini, this is only testing >=1.11.
Codecov Report
@@ Coverage Diff @@ ## master #411 +/- ## ========================================== - Coverage 91.79% 91.75% -0.04% ========================================== Files 55 55 Lines 2925 2923 -2 ========================================== - Hits 2685 2682 -3 - Misses 240 241 +1
Continue to review full report at Codecov.
|
@mblayman Looks like now it's safe to merge. I had a bit of a learning curve with tox, travis, etc. manage.py test still fails though the travis and tox tests don't catch it.
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 pretty good. I'd like to understand how the DJA package is made available to example app. Did you happen to run the steps you listed in Getting Started from a totally fresh clone to see if it would start the example app properly?
Once I have a bit more confidence that these steps will install DJA for the example app, I'm happy to merge.
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.
How does this work? I don't understand where the DJA package would be available to Python if it's not installed in editable mode.
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.
It's available because you are in the source tree for DJA so import rest_framework_json_api just works. I suppose retaining pip install -e . can't hurt. Pushing a new commit.
django-rest-framework-json-api$ tree -L 1
.
├── AUTHORS
├── CHANGELOG.md
├── LICENSE
├── MANIFEST.in
├── README.rst
├── docs
├── drf_example
├── env
├── example
├── manage.py
├── pytest.ini
├── requirements-development.txt
├── requirements.txt
├── rest_framework_json_api
├── setup.cfg
├── setup.py
└── tox.ini
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.
Sorry, neglected to answer the question: "Did you happen to run the steps you listed in Getting Started from a totally fresh clone to see if it would start the example app properly?"
Yes. Several times.
Thanks! I appreciate all your work on this branch. I forgot to ask you to add your name to the AUTHORS file. We do that to give you credit for your contribution and recognize that you have some copyright claim to this project. The list is alphabetical by first name in case that's somehow unclear.
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.
Thanks @n2ygk!
I was a bit confused by the various sources of urlpatterns, etc. and hopefully got it right. I ran both the example app with runserver and did a tox run too to make sure I didn't break that.
For whatever reason, I was unable to get this to work by doing
export DJANGO_SETTINGS_MODULE=example.settingsand variousdjango-admin.pystuff as I got an error about module example not found. I'm not quite sure why that happened.However, doing basically the same thing by creating
manage.pyworks and, sincemanage.pyis the usual documented approach, I think that's OK.It's a little different from the usual pattern in that there's not a project directory with settings and then an example app directory; both are the same, but that's OK I think.
There's still one problem in here in that
http://127.0.0.1:8000/entries/2/relationships/suggestedreturns a 404. I must have missed something. @mblayman Maybe you can find it quicker than I can.