Page MenuHomePhabricator

autopep8 various files
ClosedPublic

Authored by deadalnix on Aug 4 2017, 19:47.

Details

Summary

As per title.

Test Plan

Run the tests after fixing them in D413 .

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Honestly I don't see the huge advantage in this coding standard of 80 character lines. It really just makes the code uglier.

I mean look at some of the stuff the autoformatter did -- it basically made readable function calls look.. strange.. all just to strangle them into fitting on an 80 char line. I question the wisdom of making easily readable code look bizarre in favor of 80 characters. I don't think it's a wise tradeoff to make and .. aesthetically it's displeasing.

I had similar issues with my C++ code on one of the diffs I submitted. You basically didn't like it because the autoformatter was unhappy.

I say: We can and should intelligently ignore the autoformatter when we want to, and rather just use it as a reminder we are free to ignore.

In this diff there are tons of places where the lines it formtted look a lot worse and less readable than what was originally there.

Do we have to do this?

I suggest you read this paragraph from the Pep8 style guide:

https://www.python.org/dev/peps/pep-0008/#a-foolish-consistency-is-the-hobgoblin-of-little-minds

Basically even the Pep8 style guide authors acknowledge blindly following a style guide is not always wise, and you should use your best judgement.

qa/rpc-tests/mempool_packages.py
46 ↗(On Diff #1031)

Shouldn't this comment go before the asset?

qa/rpc-tests/mempool_packages.py
28 ↗(On Diff #1031)

This looks less readable than the original code.

Why are we doing this? Do we worship autopep8 now?

Runs ok. Note some tests still fail but fewer fail with this series of diffs: D409, D411, D413, D414, so I will accept them since they are a step in the right direction...

Also D417 depends on these..

This revision is now accepted and ready to land.Aug 5 2017, 06:56
This revision was automatically updated to reflect the committed changes.