Skip to content

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 21, 2016
Merged

Fix #852, anyway this parameter never used #855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e7b0e0
Fix #852, anyway this parameter never used
valignatev Jul 20, 2016
cfc0aa4
#852 Update minimum sqlparse version to 0.2
valignatev Jul 20, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion debug_toolbar/panels/sql/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

class BoldKeywordFilter:
"""sqlparse filter to bold SQL keywords"""
def process(self, stack, stream):
def process(self, stream):
"""Process the token stream"""
for token_type, value in stream:
is_keyword = token_type in T.Keyword
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
packages=find_packages(exclude=('tests.*', 'tests', 'example')),
install_requires=[
'Django>=1.8',
'sqlparse',
'sqlparse>=0.2.0',
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor

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.)

Copy link
Contributor

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.)

Copy link
Contributor

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.

],
include_package_data=True,
zip_safe=False, # because we're including static files
Expand Down