-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #852, anyway this parameter never used #855
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
Conversation
@@ -16,7 +16,7 @@ | |||
packages=find_packages(exclude=('tests.*', 'tests', 'example')), | |||
install_requires=[ | |||
'Django>=1.8', | |||
'sqlparse', | |||
'sqlparse>=0.2.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'd add upper limit as well:
'sqlparse >=0.2.0,<0.3',
This will help to avoid similar issues with unexpected API changes.
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 don't think sqlparse strictly adheres to semantic versioning and I'm afraid we'll forget to update this when 0.3 gets release, so I prefer the current version.
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.
@aaugustin
https://requires.io/ could be used to track dependencies status. They provide a badge, which can be included in README.md
, so if some package gets outdated it changes background from green to yellow or red alerting the issue.
They also list all packages with their versions like this: https://requires.io/github/celery/celery/requirements/?branch=5.0-devel
This way, it's easy to mention one requiring update.
There's also an option to be notified about outdated packages by email and/or pull request.
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'll submit a couple PRs suggesting to add useful badges if you don't mind
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.
Since we don't set upper limits, how could our requirements get out of date?
(Also I'm not a fan of cargo-culting badges.)
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.
(And I'm especially not a fan of shaming badges that become red if I don't look at the repo for a month or two.)
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.
Well it makes sense to consider changing personal preferences given that there's a tool to help handle this. The badge motivates to keep it green, but it's not a must. I believe, automatic notifier does not require adding badge explicitly to work.
Still it's up to you to decide whether to agree with this point.
@webknjaz I'd rather not. Why lock ourselves to old version of other packages? We can also add upper boundary for Django that way. Let's say, <1.10. |
@valentjedi it will still allow to patch version upgrades. It looks they don't follow semver, introducing backwards incompatible changes with minor version upgrade. So only upgrading to patch version change is safe. |
Fix django-commons#852, anyway this parameter never used
Fix for new process api of sqlparse.