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

Unignore even more style checks #1579

Merged
merged 6 commits into from
Jun 30, 2019
Merged

Conversation

half-duplex
Copy link
Member

@half-duplex half-duplex commented Apr 30, 2019

Unignores (or updates to add) and fixes the following:
E117 over-indented
E127 continuation line over-indented for visual indent
E128 continuation line under-indented for visual indent
F632 use ==/!= to compare str, bytes, and int literals

With the above fixed, flake8 3.7.7 finds no errors, so the 3.6 pin is replaced with 3.7

checkstyle.sh's py3-unsafe unicode() detection seemed to have some inverted logic and was reporting files containing unicode = str as potentially unsafe, instead of only files not containing that.

@half-duplex half-duplex added this to the 7.0.0 milestone Apr 30, 2019
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

I'm not 100% on board with the multi-lines if change, but otherwise this looks pretty good.

sopel/modules/dice.py Outdated Show resolved Hide resolved
sopel/modules/dice.py Outdated Show resolved Hide resolved
@half-duplex half-duplex force-pushed the checkstyle-updates branch from d9704a7 to d2847a8 Compare May 1, 2019 20:35
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

🎉

@Exirel
Copy link
Contributor

Exirel commented May 8, 2019

@half-duplex aw, now there are conflict to fix first. :(

@half-duplex half-duplex force-pushed the checkstyle-updates branch 4 times, most recently from 3d2b41c to 6a2a813 Compare May 12, 2019 07:23
@half-duplex half-duplex requested a review from Exirel May 12, 2019 07:27
@half-duplex
Copy link
Member Author

Had to make the same changes in some new code after the rebase

Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

Just a question, because I don't remember having an issue with single/double quote string before.

sopel/coretasks.py Show resolved Hide resolved
test/modules/test_modules_remind.py Show resolved Hide resolved
@half-duplex half-duplex force-pushed the checkstyle-updates branch 3 times, most recently from 204c623 to 853ce79 Compare June 3, 2019 20:07
@dgw
Copy link
Member

dgw commented Jun 22, 2019

Of course merging #1578 caused conflicts with this PR, because why wouldn't it? 😑

@half-duplex, would you fix this one up for me over the weekend? 😸

@Exirel
Copy link
Contributor

Exirel commented Jun 22, 2019

Oh, that's a very easy conflict to fix! :)

@dgw
Copy link
Member

dgw commented Jun 30, 2019

Tested merging this to a local branch, and I'm gonna merge it before anything breaks the newly fixed checks. I want better code style just like everyone else!

@dgw dgw merged commit 9340c67 into sopel-irc:master Jun 30, 2019
@half-duplex half-duplex deleted the checkstyle-updates branch July 1, 2019 06:38
@dgw
Copy link
Member

dgw commented Oct 11, 2019

Me from the future would like to ask himself from the past why he didn't insist on reordering things to prevent a commit with failed tests from being merged into master… Oh well.

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

Successfully merging this pull request may close these issues.

3 participants