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

Formatting with black #204

Merged
merged 5 commits into from
Aug 30, 2018
Merged

Formatting with black #204

merged 5 commits into from
Aug 30, 2018

Conversation

max-sixty
Copy link
Contributor

As an example of how the repo would look using https://github.com/ambv/black

I find this great for repos I'm starting. I'm not sure it's worth the tradeoff of merge conflicts for existing projects, though.

pandas_gbq/auth.py Show resolved Hide resolved
pandas_gbq/gbq.py Show resolved Hide resolved
pandas_gbq/gbq.py Show resolved Hide resolved
pandas_gbq/gbq.py Show resolved Hide resolved
assert len(df.drop_duplicates()) == 10

def test_standard_sql(self, project_id):
standard_sql = "SELECT DISTINCT id FROM " \
"`publicdata.samples.wikipedia` LIMIT 10"
standard_sql = "SELECT DISTINCT id FROM " "`publicdata.samples.wikipedia` LIMIT 10"

Choose a reason for hiding this comment

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

E501 line too long (91 > 79 characters)

Copy link
Collaborator

@tswast tswast left a comment

Choose a reason for hiding this comment

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

I'd be happy to update to this workflow and am okay with the one-time cost of a few (or a lot) of merge conflicts.

We could probably change the "lint" session in nox.py to run changes through black and complain if there's any diff afterward. That pattern is pretty common in Go projects I've worked on.

@max-sixty
Copy link
Contributor Author

OK great, I can add that to CI.

Any thoughts @parthea @jreback, given this is a v broad change?

@parthea
Copy link
Contributor

parthea commented Aug 30, 2018

Sounds good to me. Thanks for adding this, @max-sixty !

@max-sixty max-sixty changed the title [No merge] Formatting with black Formatting with black Aug 30, 2018
@max-sixty
Copy link
Contributor Author

@tswast please could you take a final look before I merge? I changed Stickler to use black rather than running in Travis on nox

Copy link
Collaborator

@tswast tswast left a comment

Choose a reason for hiding this comment

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

Thanks!

@max-sixty max-sixty merged commit 294c924 into googleapis:master Aug 30, 2018
@max-sixty max-sixty deleted the black branch August 30, 2018 20:23
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