Skip to content

Add Sphinx as a dependency for linting #2019

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 22, 2016
Merged

Add Sphinx as a dependency for linting #2019

merged 1 commit into from
Jul 22, 2016

Conversation

@daspecster
Copy link
Contributor

@daspecster daspecster commented Jul 22, 2016

First seen #1946 (comment)

screen shot 2016-07-22 at 6 10 03 pm

Not sure how this got through?

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 22, 2016
@daspecster daspecster force-pushed the fix-lint-sphinx-dep branch from e3a5018 to 554285b Compare July 22, 2016 22:11
@daspecster daspecster mentioned this pull request Jul 22, 2016
Merged
@waprin
Copy link
Contributor

waprin commented Jul 22, 2016

lgtm

@daspecster daspecster merged commit 04a2543 into googleapis:master Jul 22, 2016
@dhermes
Copy link
Contributor

dhermes commented Jul 22, 2016

I am 👎 on the quick merge without a more in-depth investigation. I'm pretty sure this snuck in because our [pep8] settings aren't getting picked up after the switch to pycodestyle

@daspecster
Copy link
Contributor Author

daspecster commented Jul 22, 2016 •
edited
Loading

Sorry, I can revert. I looked and the offending line is https://github.com/GoogleCloudPlatform/gcloud-python/blob/master/scripts/verify_included_modules.py#L119

Which is called from https://github.com/GoogleCloudPlatform/gcloud-python/blob/master/tox.ini#L153 actually and not the pep8 stuff AFAICT.

@waprin
Copy link
Contributor

waprin commented Jul 22, 2016

Also sorry if you think it was too hasty but it did fix my PR.

@daspecster
Copy link
Contributor Author

daspecster commented Jul 22, 2016

Also, by [pep8] settings you're referring to in the tox.ini (https://github.com/GoogleCloudPlatform/gcloud-python/blob/master/tox.ini#L144). Switching it to [pycodestyle] produces a bunch of errors like it's not picking it up for some reason.

Might be a pycodestyle bug? I tried it before when I did the pep8->pycodestyle conversion and it broke then too.

@daspecster daspecster mentioned this pull request Jul 22, 2016
Merged
@daspecster
Copy link
Contributor Author

daspecster commented Jul 22, 2016

We can talk more here #2020

@tseaver
Copy link
Contributor

tseaver commented Jul 23, 2016 •
edited
Loading

That is a failure in pylint, not pycodestyle: it is trying to check docs/conf.pyscripts/verify_included_modules.py. We could just exclude that file from pylint.

daspecster added a commit to daspecster/google-cloud-python that referenced this pull request Jul 23, 2016
@daspecster
Revert googleapis#2019.
75aae3b
@daspecster daspecster mentioned this pull request Jul 23, 2016
Merged
@daspecster
Copy link
Contributor Author

daspecster commented Jul 23, 2016

@tseaver thanks! Here we go #2021.

This was referenced Aug 3, 2016
Closed
Merged
@daspecster daspecster deleted the fix-lint-sphinx-dep branch January 24, 2017 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@tseaver tseaver

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@daspecster @waprin @dhermes @tseaver @googlebot