Skip to content
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

Missing import in check_location #526

Merged
merged 4 commits into from
Jul 7, 2018
Merged

Missing import in check_location #526

merged 4 commits into from
Jul 7, 2018

Conversation

sww314
Copy link
Contributor

@sww314 sww314 commented Jul 6, 2018

Fix missing import needed in #520

@codecov-io
Copy link

codecov-io commented Jul 6, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@09ea828). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #526   +/-   ##
=========================================
  Coverage          ?   76.75%           
=========================================
  Files             ?       11           
  Lines             ?     1596           
  Branches          ?        0           
=========================================
  Hits              ?     1225           
  Misses            ?      371           
  Partials          ?        0
Impacted Files Coverage Δ
storages/utils.py 96.96% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09ea828...e47de76. Read the comment docs.

@jschneier
Copy link
Owner

@jdufresne @sww314 how do we feel about codecov? I added it in a pique of interest at some point, anything you guys think works better or that we are missing?

from django.utils.encoding import force_text



Copy link
Contributor

Choose a reason for hiding this comment

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

This extra new line is causing flake8 to fail. If you remove it Travis should be green again.

@jdufresne
Copy link
Contributor

@jschneier I have no strong opinion. I think most PRs -- especially features and bug fixes -- should come with tests. But I don't find the coverage report all the useful personally. I'm ok either way.

@sww314 sww314 merged commit 950d2a9 into master Jul 7, 2018
@sww314
Copy link
Contributor Author

sww314 commented Jul 7, 2018

@jschneier I agree with Jon. Tests are critical for features and bug fixes. I do not pay much attention to the code coverage report.

@sww314 sww314 deleted the location-patch branch July 7, 2018 04:28
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.

4 participants