-
Notifications
You must be signed in to change notification settings - Fork 36
Django 2.2 & 3.0 #65
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
Django 2.2 & 3.0 #65
Conversation
| packages=['django_coverage_plugin'], | ||
| install_requires=[ | ||
| 'coverage >= 4.0', | ||
| 'coverage >= 4.0 , < 5', |
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.
Why do this?
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.
From the test output of the first commit:
Traceback (most recent call last):
File "tests/test_extends.py", line 16, in test_empty_block
text = self.run_django_coverage()
File "tests/plugin_test.py", line 139, in run_django_coverage
for pl in self.cov.plugins:
AttributeError: 'Coverage' object has no attribute 'plugins'I've been planning to switch to coverage.CoveragePlugin, but I've been swamped at work.
EDIT: The end of my comment about CoveragePlugin is misleading. Please ignore it!
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.
Yes, was failing without. I feel like it's out of scope of this PR?
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'm in agreement.
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.
OK: #66
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.
Given that this is broken on master (I think), should I move this to a separate pull request so you can make a patch release of this plugin before adding more stuff?
The FILE_CHARSET setting is removed.
0ad4215 to
bffef00
Compare
jambonrose
left a comment
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.
Thank you for the PR!
Please update the Read Me to note the new Python & Django versions officially being supported.
8688e66 to
ab5fb5c
Compare
|
@jambonrose I think I made all the changes you requested. Let me know if you see anything else. |
jambonrose
left a comment
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 good to me! Thank you for taking to time to work on this plugin.
Uh oh!
There was an error while loading. Please reload this page.