Skip to content

Enable pylint and flake8 warnings - #73

Merged
p12tic merged 3 commits into
masterfrom
enable-pylint-flake8
Oct 24, 2018
Merged

p12tic merged 3 commits into
masterfrom
enable-pylint-flake8

Conversation

@p12tic

@p12tic p12tic commented Oct 24, 2018

Copy link
Copy Markdown
Owner

No description provided.

@p12tic
p12tic force-pushed the enable-pylint-flake8 branch 3 times, most recently from e019027 to 0aa089f Compare October 24, 2018 19:31
@p12tic
p12tic force-pushed the enable-pylint-flake8 branch from 0aa089f to ee6e929 Compare October 24, 2018 19:35
@p12tic p12tic changed the title Enable pylint flake8 warnings Enable pylint and flake8 warnings Oct 24, 2018
@p12tic
p12tic merged commit 3a39488 into master Oct 24, 2018
@PeterFeicht

Copy link
Copy Markdown
Contributor

Aw man, I really wish you hadn't chosen such a small line length limit. 80 characters just seems unnecessary these days :-/
Any chance you might consider upping that?

@p12tic

p12tic commented Oct 25, 2018

Copy link
Copy Markdown
Owner Author

It turned out that 80 characters is almost the maximum that can be displayed side by side without compromising readability on my editor of choice on a full HD monitor. So for this reason I'd like to stick to 80 characters. Myself I do find it limiting sometimes, but not enough to consider to switch. Sorry for declining your suggestion.

@PeterFeicht

Copy link
Copy Markdown
Contributor

Alright, I'll learn to live with it I guess :-)
Did you do the formatting manually, or was that done by a tool?

@p12tic

p12tic commented Nov 9, 2018

Copy link
Copy Markdown
Owner Author

There's autopep8, but I found it wraps lines at "wrong" places, so this time I did it manually.

@PeterFeicht

Copy link
Copy Markdown
Contributor

I'm asking because I thought a few things were wrapped very weirdly. Would you be open to a couple small changes in that regard?

@p12tic

p12tic commented Nov 9, 2018

Copy link
Copy Markdown
Owner Author

Could you give some links?

@PeterFeicht

Copy link
Copy Markdown
Contributor

preprocess.py:131 or preprocess.py:224 for example, I think they'd look better when wrapped earlier. Or in preprocess_qch.py I'd move the enumerate into the next loop, since the index is not needed before that.

@p12tic

p12tic commented Nov 9, 2018

Copy link
Copy Markdown
Owner Author

Agreed.

@PeterFeicht

Copy link
Copy Markdown
Contributor

Great, then I'll do a PR sometime.

@p12tic
p12tic deleted the enable-pylint-flake8 branch June 5, 2019 20:01
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.

2 participants