-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
All package updates #4792
All package updates #4792
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4792 +/- ##
==========================================
+ Coverage 76.75% 76.75% +<.01%
==========================================
Files 158 158
Lines 10048 10051 +3
Branches 1265 1265
==========================================
+ Hits 7712 7715 +3
Misses 1995 1995
Partials 341 341
|
@humitos now that we are on a recent version of django, we can revert the last commit p: or maybe upgrade the rest of packages too! |
2f25436
to
5b9195a
Compare
@stsewd thanks for noting this. I've updated all the packages and followed the migration guides that we had noted in the requirements files. I think that all tests should pass. |
5b9195a
to
1c6a093
Compare
requirements/pip.txt
Outdated
# Besides the guide, check this comment for the migration | ||
# https://github.com/rtfd/readthedocs.org/pull/4318#discussion_r214163531 | ||
django-filter==1.1.0 | ||
django-filter==2.0.0 |
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.
If we want to upgrade to django-filter>=2.0.0
we need to drop support for Python2 since it requires Python3. Tests fail with:
> from .filterset import FilterSet
E File "/home/travis/build/rtfd/readthedocs.org/.tox/py27/lib/python2.7/site-packages/django_filters/filterset.py", line 184
E def __init__(self, data=None, queryset=None, *, request=None, prefix=None):
E ^
E SyntaxError: invalid syntax
e3ad2a2
to
1c2034e
Compare
requirements/deploy.txt
Outdated
@@ -2,8 +2,8 @@ | |||
# http://initd.org/psycopg/docs/install.html#binary-install-from-pypi | |||
psycopg2==2.7.5 --no-binary psycopg2 | |||
gunicorn==19.9.0 | |||
pysolr==3.7.0 | |||
django-redis-cache==1.7.1 | |||
pysolr==3.8.1 |
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 like something used for hystack only https://github.com/django-haystack/pysolr, it was removed a time ago #4039
requirements/pip.txt
Outdated
# Required to avoid Transifex error with reserved slug | ||
# https://github.com/sphinx-doc/sphinx-intl/pull/27 | ||
git+https://github.com/agjohnson/sphinx-intl.git@7b5c66bdb30f872b3b1286e371f569c8dcb66de5#egg=sphinx-intl | ||
|
||
Pygments==2.2.0 | ||
|
||
mkdocs==0.17.3 | ||
mkdocs==1.0.4 | ||
# mkdocs requires markdown | ||
# Markdown 3.0 breaks with older Django Rest Framework | ||
Markdown<3.0 |
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.
Maybe we are safe to update this?
# E def __init__(self, data=None, queryset=None, *, request=None, prefix=None): | ||
# E ^ | ||
# E SyntaxError: invalid syntax | ||
django-filter<2.0.0 |
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.
I guess the above comments
filter_fields = ('slug',) # django-filter<2.0.0
filterset_fields = ('slug',)
are because of this? I think is better to just create an issue, rather than hiding it on the code
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.
We can't upgrade to 2.0.0 yet because we are still running tests with Py2. Once we remove that, we can upgrade without touching the code.
The comments added into the code already prepared it for 2.0.0.
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.
I prefer to prepare the code to work with both versions now and when we upgrade, we can remove the lines that are not needed.
d810291
to
f34d746
Compare
$ for reqs in `ls requirements`; do pur --skip django,docker,elasticsearch,pyelasticsearch,commonmark,stripe,mkdocs,gitpython,mercurial --requirement requirements/$reqs; done
$ for reqs in `ls requirements`; do pur --skip django,docker,celery,gitpython,elasticsearch,pyelasticsearch,mercurial --requirement requirements/$reqs; done
$ for reqs in `ls requirements`; do pur --skip stripe,commonmark,django,docker,celery,gitpython,elasticsearch,pyelasticsearch,mercurial --requirement requirements/$reqs; done
$ for reqs in `ls requirements`; do pur --skip django,docker,celery,commonmark,gitpython,elasticsearch,pyelasticsearch,mercurial --requirement requirements/$reqs; done
f34d746
to
f0edd22
Compare
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.
Looks good to me. I think we should merge this and update locally to test it before it gets deployed.
Closes #4897