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

Various PEP8 Fixes #6156

Closed
wants to merge 1 commit into from
Closed

Various PEP8 Fixes #6156

wants to merge 1 commit into from

Conversation

super3
Copy link
Contributor

@super3 super3 commented May 18, 2015

@super3
Copy link
Contributor Author

super3 commented May 18, 2015

About 600+ PEP8 style violations in /contrib alone. Just fixed the comma spacing errors first.

@laanwj
Copy link
Member

laanwj commented May 19, 2015

NACK. Don't do this - these kinds of floods of minor whitespace changes break other pulls for no good reason.

@super3
Copy link
Contributor Author

super3 commented May 19, 2015

@laanwj All of PEP8 violations I'm covering are in /contrib. With the exception of /contrib/debian these directories have not been touched for 4+ months. I agree that minor floods of whitespace on active code would cause problems, but that directory is far from active.

@super3
Copy link
Contributor Author

super3 commented May 19, 2015

Guess the other question is should Bitcoin code even be PEP8 compliant?

@jonasschnelli
Copy link
Contributor

I also tend to NACK this.
Generally IMO clean code makes sense. But, at the moment there are many pull requests and it's hard to keep focus on what is going on.
IMO your PR does not produce a benefit for this project at first place.
Somebody needs to review this. This requires time which is probably better spent at some other PRs.

I think this PR could be something for the trivial branch https://github.com/theuni/bitcoin.
This goes also in the same direction as the clang-everything PR (#5377, #5387)

@laanwj
Copy link
Member

laanwj commented May 19, 2015

Some PEP8 "violations" are worse than others. E.g. using the wrong kind of indentation would be awful.

But adding spaces to almost every line so that ',' is lined up. No, that's not important.
Please don't waste your time on this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants