-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
[flake8] Resolving Q??? errors #3847
Conversation
ab21b5f
to
6d01b69
Compare
Agreed, single over double quotes. Personally I prefer switching to avoid backslashes but I feel bad making you make lots of edits. One thing though is that I prefer not enforcing rules that linters cannot enforce, so can we get pylint to enforce it? If not we'll have to tell each person on their first PR which is annoying both on the reviewer and reviewee side... |
Looks like this would do it: |
@mistercrunch I can switch from single to double quotes for the strings which include quotes to avoid backslashes. I'll comment on the PR when the change is made. |
Sweet that will be ideal. It's also consistent with how the JS linter is set up. |
@mistercrunch |
3b7251c
to
607d446
Compare
607d446
to
caaee0d
Compare
@mistercrunch I've update the PR to remove all occurrences of backslashes in the string. |
Woot! |
(cherry picked from commit ac57780)
) | ||
|
||
@classmethod | ||
def convert_dttm(cls, target_type, dttm): | ||
tt = target_type.upper() | ||
if tt == 'DATE': | ||
return "'{}'".format(dttm.strftime('%Y-%m-%d')) | ||
return "{}'".format(dttm.strftime('%Y-%m-%d')) |
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 caused: #4915
) | ||
|
||
@classmethod | ||
def convert_dttm(cls, target_type, dttm): | ||
tt = target_type.upper() | ||
if tt == 'DATE': | ||
return "'{}'".format(dttm.strftime('%Y-%m-%d')) | ||
return "{}'".format(dttm.strftime('%Y-%m-%d')) |
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 caused: #4915
Caused by apache#3847 Fixes apache#4915
Caused by apache#3847 Fixes apache#4915
Caused by apache#3847 Fixes apache#4915
Caused by apache#3847 Fixes apache#4915
(cherry picked from commit 067596e)
This is the final PR related to remedying the flake8 errors. This resolves the flake8 Q??? violation errors which enforces single quotes for strings.
Previously there was a large mix of single and double quotes used for strings, and per PEP8,
it mentions to pick a rule and stick to it. Given that the majoring of strings were single-quoted I used that as the default. Note some strings include
'
which I've escaped as opposed to using the other to avoid backslashes. I'm happy to remedy this if necessary.to: @mistercrunch