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

Remove warnings from code #3372

Merged
merged 6 commits into from
Dec 21, 2017
Merged

Remove warnings from code #3372

merged 6 commits into from
Dec 21, 2017

Conversation

humitos
Copy link
Member

@humitos humitos commented Dec 7, 2017

This PR removes almost all the WARNING messages when running tests:

  • migrate render_to_response to render from Django
  • use prepend_urls instead override_urls from tastypie

There are two more warnings:

  1. Tastypie uses an old method from Django, but we can't upgrade (even from 0.13.0 to 0.13.3) because there are some tests that do not pass (I could take a look at this in another PR)
readthedocs/rtd_tests/tests/test_api.py::APITests::test_make_project
  /home/humitos/rtfd/code/readthedocs.org/.tox/py27/lib/python2.7/site-packages/tastypie/resources.py:2116: RemovedInDjango110Warning: 'get_all_field_names is an unofficial API that has been deprecated. You may be able to replace it with 'get_fields()'
    field_names = self._meta.object_class._meta.get_all_field_names()
  1. Linaro Django Pagination uses context_instance for render_to_string. I took a look at the code but we are using an specific commit directly from git and I found that project was forked and abandoned a couple of times. So, I'm not sure if it's safe to update and to what version
readthedocs/rtd_tests/tests/test_privacy.py::PrivacyTests::test_build_filtering
  /home/humitos/rtfd/code/readthedocs.org/.tox/py27/lib/python2.7/site-packages/linaro_django_pagination/templatetags/pagination_tags.py:199: RemovedInDjango110Warning: The context_instance argument of render_to_string is deprecated.
    context_instance = context)

@ericholscher
Copy link
Member

  1. We should deprecate our tastypie API and remove it.
  2. Not sure about.

This is a solid 👍 from me once the tests pass :)

@humitos
Copy link
Member Author

humitos commented Dec 7, 2017

Test are passing, the problem is the linting. I wasn't sure if I should apply all the tools we have to this files or not.

Since, you already review it, I could apply them and make a new commit with all of the files involved.

  1. and 2) could be in another PR I think.

@humitos humitos force-pushed the humitos/migrate/warnings branch from c6a3fb7 to eef9f65 Compare December 12, 2017 02:22
@humitos humitos force-pushed the humitos/migrate/warnings branch from cd29ebd to b614df0 Compare December 19, 2017 02:33
@humitos humitos force-pushed the humitos/migrate/warnings branch from b614df0 to 7fd9a01 Compare December 21, 2017 02:45
@ericholscher
Copy link
Member

LGTM. Will likely cause a bunch of other PR's to conflict though :(

@ericholscher ericholscher merged commit a84b5ed into master Dec 21, 2017
@humitos humitos mentioned this pull request Dec 26, 2017
23 tasks
@stsewd stsewd deleted the humitos/migrate/warnings branch August 15, 2018 22:15
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.

2 participants