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

[dev/tooling] Enforce single quote strings #884

Merged
merged 10 commits into from
Apr 16, 2019

Conversation

Kyle-Verhoog
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog commented Apr 13, 2019

Finally this PR adds a flake8 plugin (https://github.com/zheller/flake8-quotes) to enforce single quote usage.

brettlinter

This removes the dependency we had on the brettlinter 😄.

This PR also replaces double quote strings with the corresponding single quote string. Strings containing single quotes remained untouched (which is permitted in the plugin https://github.com/zheller/flake8-quotes#caveats). We may want to figure out our convention for these cases.

Note: update branch to master once #881 has been merged.

@Kyle-Verhoog Kyle-Verhoog added this to the 0.25.0 milestone Apr 13, 2019
@Kyle-Verhoog Kyle-Verhoog requested a review from a team as a code owner April 13, 2019 07:52
Copy link
Contributor

@jd jd left a comment

Choose a reason for hiding this comment

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

LGTM.

Pretty sure it'll hurt me as I'm used to " but if you want to have consistency, I'm fine with it.

@brettlangdon
Copy link
Member

@Kyle-Verhoog you are my hero!

@jd yeah, I've been trying to keep consistent on ', figure pick one and go with it, if we ever decided to move to black or something to do auto-formatting, then we'll just go with whatever it picks

@brettlangdon brettlangdon changed the base branch from 0.24-dev to 0.25-dev April 15, 2019 17:12
@brettlangdon brettlangdon merged commit 7fd81a5 into 0.25-dev Apr 16, 2019
@brettlangdon brettlangdon deleted the kyle-verhoog/rm-brettlinter branch April 16, 2019 13:25
@majorgreys majorgreys mentioned this pull request Apr 30, 2019
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.

3 participants