Skip to content

Conversation

brunobord
Copy link
Contributor

@brunobord brunobord commented Mar 28, 2017

Review

This PR closes #203

  • Tests (added in tox/circle-ci)
  • CHANGELOG.rst Updated
  • Delete your branch

mgu
mgu previously approved these changes Mar 28, 2017
Copy link
Contributor

@mgu mgu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@brunobord
Copy link
Contributor Author

@mgu you misread. the tox.ini is wrong.

@brunobord brunobord dismissed mgu’s stale review March 28, 2017 08:50

the tox.ini was wrong (see fixup)

@mgu
Copy link
Contributor

mgu commented Mar 28, 2017

you are right, I missed your copy/paste error

@brunobord
Copy link
Contributor Author

dammit, something is wrong here:

tox -re django110-py27
[snip snip snip]
django110-py27 runtests: commands[1] | pip list --format=columns
Package                            Version    Location                                          
---------------------------------- ---------- --------------------------------------------------
appdirs                            1.4.3      
backports.shutil-get-terminal-size 1.0.0      
decorator                          4.0.11     
Django                             1.9.12     
[....]

@brunobord
Copy link
Contributor Author

ok, got it, it was locked in the setup.py

@brunobord brunobord force-pushed the 203-django1.10-support branch from 7353880 to 05fb8b8 Compare March 28, 2017 09:20
Copy link
Contributor

@mgu mgu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@brunobord brunobord merged commit 5a192c5 into master Mar 29, 2017
@brunobord brunobord deleted the 203-django1.10-support branch March 29, 2017 13:31
'guillaume.gerard@people-doc.com',
install_requires=[
'Django<1.10',
'Django',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have set Django<1.11 here instead. Because, it'll install the very latest version of Django here if it's unpinned. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this silly requirement has almost make me lose my mind ; I was working with tox "django110" jobs, and it was still Django 1.9 that was installed. Luckily I did a careful check to the "pip freeze -l" that is executed during the job, or we could have a "fake" Django 1.10 support.

Maybe we could think about having a "latest" job, that installs the latest Django version, run it through a cron job (I don't know if circle-ci is able to do it, but travis does) and periodically check if the latest django release is compatible, in order to anticipate further releases.

@wo0dyn
Copy link
Contributor

wo0dyn commented Mar 30, 2017

Maybe we could think about having a "latest" job, that installs the latest Django version, run it through a cron job (I don't know if circle-ci is able to do it, but travis does) and periodically check if the latest django release is compatible, in order to anticipate further releases.

Yeah, that would be awesome indeed.
Thanks for your explanation anyway.

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.

Add django 1.10 support

3 participants