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

Issue 190 #279

Merged
merged 2 commits into from Aug 6, 2016
Merged

Issue 190 #279

merged 2 commits into from Aug 6, 2016

Conversation

phdru
Copy link
Contributor

@phdru phdru commented Aug 5, 2016

Fix #190.

@coveralls
Copy link

coveralls commented Aug 5, 2016

Coverage Status

Coverage decreased (-1.1%) to 96.201% when pulling 9474db4b752f57c732f9c1e4f2babd62ea45ce66 on phdru:issue-190 into b7e6ce7 on andialbrecht:master.

@coveralls
Copy link

coveralls commented Aug 5, 2016

Coverage Status

Coverage increased (+0.01%) to 97.348% when pulling 02c29dc8f14d4495e06ab1cb6bcc2802943d63a8 on phdru:issue-190 into b7e6ce7 on andialbrecht:master.

@codecov-io
Copy link

codecov-io commented Aug 5, 2016

Current coverage is 97.34% (diff: 100%)

Merging #279 into master will increase coverage by <.01%

@@             master       #279   diff @@
==========================================
  Files            21         21          
  Lines          1390       1394     +4   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1353       1357     +4   
  Misses           37         37          
  Partials          0          0          

Powered by Codecov. Last update b7e6ce7...44e6671

@@ -20,6 +20,13 @@
from sqlparse.utils import consume


try:
file
Copy link
Contributor

Choose a reason for hiding this comment

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

whats the purposed of this line ?

@vmuriart
Copy link
Contributor

vmuriart commented Aug 5, 2016

Can you add a couple tests here as well

@phdru
Copy link
Contributor Author

phdru commented Aug 5, 2016

To distinguish Python2 and Python3:

$ python3
Python 3.4.2 (default, Oct  8 2014, 13:14:40) 
[GCC 4.9.1] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> file
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'file' is not defined

@vmuriart
Copy link
Contributor

vmuriart commented Aug 5, 2016

Wouldn't the next line do the same thing? It should still raise the same Exception (never tried it though)

@phdru
Copy link
Contributor Author

phdru commented Aug 5, 2016

Oh, yes. 😄

@coveralls
Copy link

coveralls commented Aug 5, 2016

Coverage Status

Coverage increased (+0.008%) to 97.346% when pulling dd0581c62a746ec790ebad77e15ae7485dc3884e on phdru:issue-190 into b7e6ce7 on andialbrecht:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 97.346% when pulling 44e6671 on phdru:issue-190 into b7e6ce7 on andialbrecht:master.

1 similar comment
@coveralls
Copy link

coveralls commented Aug 5, 2016

Coverage Status

Coverage increased (+0.008%) to 97.346% when pulling 44e6671 on phdru:issue-190 into b7e6ce7 on andialbrecht:master.

@phdru
Copy link
Contributor Author

phdru commented Aug 5, 2016

Done. Fixed and added a test. All tests passed.

@vmuriart vmuriart merged commit d999cde into andialbrecht:master Aug 6, 2016
@vmuriart
Copy link
Contributor

vmuriart commented Aug 6, 2016

Thanks for the pr!

@phdru phdru deleted the issue-190 branch August 6, 2016 16:07
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.

None yet

4 participants