FM-17 / poglink

A bot that integrates the ARK Web API with Discord.
MIT License
12 stars 4 forks source link

Travis flake8 #66

Closed travipross closed 2 years ago

travipross commented 2 years ago

Adds flake8 to the pre-commit hook, plus fixes a bunch of things that violated flake8 rules.

I'm totally fine if you want to reject this one if you feel it'd be annoying to keep up with though. Note there were a few rule exclusions I added to the .flake8 config file, as they directly contradicted black.

I do, however, like the checks for things like unused imports and bad programming practice (bare exception catching, checking == None instead of is None, etc).

There are a lot of formatting changes I've made here, and it's not immediately obvious for all of them what flake8 rule they were violating. If you were curious and wanted to familiarize, I'd recommend checking out the travis-pre-commit branch that this PR uses as its base, and then running flake8 manually.

codecov[bot] commented 2 years ago

Codecov Report

Merging #66 (b8cd5b1) into main (56321b5) will decrease coverage by 0.03%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #66      +/-   ##
==========================================
- Coverage   92.90%   92.87%   -0.04%     
==========================================
  Files          10       10              
  Lines         409      407       -2     
==========================================
- Hits          380      378       -2     
  Misses         29       29              
Impacted Files Coverage Δ
poglink/bot.py 100.00% <ø> (ø)
poglink/models/bans.py 100.00% <ø> (ø)
poglink/utils.py 100.00% <ø> (ø)
poglink/cogs/rates.py 98.01% <100.00%> (-0.04%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 56321b5...b8cd5b1. Read the comment docs.

travipross commented 2 years ago

Note the codecov report shows a super tiny reduction in test coverage, but that's just a technicality. The same 2 lines in cogs/rates.py are currently not covered by tests, but because I deleted a line or two in that file, the percentage has changed.

FM-17 commented 2 years ago

There are a lot of formatting changes I've made here, and it's not immediately obvious for all of them what flake8 rule they were violating. If you were curious and wanted to familiarize, I'd recommend checking out the travis-pre-commit branch that this PR uses as its base, and then running flake8 manually.

I've included a sample of each resulting rule violation below. If this is the expected behavior, feel free to merge.

./build/lib/poglink/cogs/rates.py:180:29: F541 f-string is missing placeholders
./build/lib/poglink/cogs/rates.py:174:80: E501 line too long (128 > 79 characters)
./build/lib/poglink/cogs/rates.py:89:88: W605 invalid escape sequence '\.'
./build/lib/poglink/cogs/rates.py:21:1: E302 expected 2 blank lines, found 1
./build/lib/poglink/cogs/rates.py:12:1: F401 'poglink.config.MIN_POLLING_DELAY' imported but unused
./build/lib/poglink/models/bans.py:115:76: W291 trailing whitespace
./build/lib/poglink/bot.py:86:13: F841 local variable 'e' is assigned to but never used
./build/lib/poglink/utils.py:42:1: W293 blank line contains whitespace
./tests/unit/test_cli.py:51:23: E712 comparison to True should be 'if cond is True:' or 'if cond:'
./tests/integration/test_cogs_rates.py:32:9: E722 do not use bare 'except' 
./tests/unit/test_models_rates.py:153:5: F811 redefinition of unused 'rates' from line 5
travipross commented 2 years ago

There are a lot of formatting changes I've made here, and it's not immediately obvious for all of them what flake8 rule they were violating. If you were curious and wanted to familiarize, I'd recommend checking out the travis-pre-commit branch that this PR uses as its base, and then running flake8 manually.

  • checked out and installed travis-pre-commit
  • installed requirements.dev.txt (to get flake8)
  • ran make format
  • ran flake8 manually

I've included a sample of each resulting rule violation below. If this is the expected behavior, feel free to merge.

Odd, I thought I replied to this first thing this morning but maybe I didn't? Anyway, yes, that looks correct. Those are the style rules that I had either fixed or chosen to ignore in the .flake8 config file. I didn't ignore much, but mostly just things that black had conflicting opinions on (like line length, and a couple line wrapping things).

github-actions[bot] commented 2 years ago

:tada: This PR is included in version 0.8.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: