Starting with flake8

[MicahWW]

By going to actions and checking out the workflow labeled 'basic fixes' you can see the beginning result of using flake8. To spare the need to pull that up, there are a few different things that flake8 is complaining about a lot of things, 399 to be specific.

1. It really doesn't like tabs instead of spaces, 345 of the errors
2. PEP8 says lines shouldn't be longer than 79 chars, 50 of the errors
3. It doesn't like having multiple statements on one line, 4 of the errors
   1. EX: if (True): break

[MicahWW]

If there has been no progress since this comment the Actions should be below.

[codyswanner]

1. It really doesn't like tabs instead of spaces, 345 of the errors

2. PEP8 says lines shouldn't be longer than 79 chars, 50 of the errors

3. It doesn't like having multiple statements on one line, 4 of the errors

   1. EX: if (True): break

Is there a way to tell it "hey for this project we're using tabs as indentation?" because I'm not particularly interesting in changing my entire setup just to make a linter happy (says the guy who nearly bends over backwards to clear PyCharm warnings... but that's different, okay?)

I do like the "line too long" warnings, I think that's a good thing for us to at least be aware of, even if in some cases we decide it's warranted. Maybe for those it would be nice to have a way to tell it "look ignore these couple lines, they HAVE to be that long, but tell me about all the others"?

For multiple statements on one line: I think I like those four lines the way they are. I can see multiple statements on one line being an issue in many cases but I think for those four lines it makes the code easier to read and follow along with.

So I guess in conclusion, I pretty much disagree with flake8 😄

[codyswanner]

I get the sense this is less about flake8 as it is about testing out GitHub actions, and that part is really cool. We might need to do a little more research to find a testing/linting suite that works for us, but as far as GitHub actions goes this is exciting 😁

[MicahWW]

After a bit of research, the most popular options are pylint and flake8 with people saying flake8 is easier to work with. I did see where people said they use both though, we could do that. Also, there is an option I know of in flake8 to not return as an error so we could give ourselves warnings.

As for your comments talking about the specific errors, there are many different options to flake8, if desired you can suppress any warning or only look for specific ones. There is also a feature to tell it to ignore a warning on a certain line, we could do that as well. I personally would like to keep tabs, it makes everything a lot easier in my opinion, I would like some sort of line limit, maybe set the limit up to 127, and I agree the multiple statements on one line helps readability.

[codyswanner]

If we can customize flake8 to not show errors/warnings we don't want, then I'm down for setting it up. I'm not opposed to running more than one linter as well if there's things pylint or another linter can add to our process.

For suppressing warnings on a specific line -- do you know how that works with line numbers changing on new commits? Does flake8 know how to deal with that, or does GitHub Actions have a way of tracking that and communicating as needed with flake8? I'm imagining a scenario where we hard-code in "suppress warnings on line 285" and then change something in init and now the lines for the entire file is different and we get that warning we were trying to suppress and potentially miss a warning/error that we would want to know about. I get the sense GitHub Actions may be capable of solving that problem, though.

[MicahWW]

Suppressing warnings is done with a comment at the end of the line, so no worry about that.

[MicahWW]

As the code stands with the max length set to:

• 79 characters (default) there are 50 lines too long
• 99 characters there are 14 lines too long
• 120 characters there are 5 lines too long
• 127 characters there are 5 lines too long

Part of me wants to aim for 99 but I don't think it really matters all that much for this project, so I am voting to go with 127.

[MicahWW]

While I believe we agree, I want to check. @codyswanner , do you agree that the warnings/errors should be suppressed for tabs over spaces and multiple statements on 1 line?

[codyswanner]

Suppressing warnings is done with a comment at the end of the line, so no worry about that.

You programmers are always so smort

[codyswanner]

As the code stands with the max length set to:

* 79 characters (default) there are 50 lines too long

* 99 characters there are 14 lines too long

* 120 characters there are 5 lines too long

* 127 characters there are 5 lines too long

Part of me wants to aim for 99 but I don't think it really matters all that much for this project, so I am voting to go with 127.

What lines are too long at 120 characters? I'm kinda thinking with that kind of length maybe we should look at trimming those down.

I'm cool with 127 if you want to set it to that, I just also want to investigate those 5 lines for why they're so long

[MicahWW]

What lines are too long at 120 characters? I'm kinda thinking with that kind of length maybe we should look at trimming those down.

Seeing as there are also 5 lines also over 127 I bet it is the same lines.

[codyswanner]

While I believe we agree, I want to check. @codyswanner , do you agree that the warnings/errors should be suppressed for tabs over spaces and multiple statements on 1 line?

I agree for tabs/spaces.

I think we should just suppress those four lines for multiple statements, at least for now. If it keeps coming up and we keep deciding that there are beneficial uses of mutli-statements then we can suppress the warnings, but I think the four lines we're keeping are more of an exception than a rule, so let's just suppress those particular lines for now.

[codyswanner]

What lines are too long at 120 characters? I'm kinda thinking with that kind of length maybe we should look at trimming those down.

Seeing as there are also 5 lines also over 127 I bet it is the same lines.

Right but what lines are those? I'm seeing four in PyCharm (that it isn't warning me about, hm), three of them are docstrings, we maybe should reformat those. One of them is the single-player prompt for choosing to move first or second, we could clean that up easy or just suppress the warning for that one line.

Where's the last one?

[MicahWW]

Errors straight from flake8:

./TicTacToe.py:94:128: E501 line too long (160 > 127 characters)
./TicTacToe.py:156:128: E501 line too long (158 > 127 characters)
./TicTacToe.py:189:128: E501 line too long (134 > 127 characters)
./TicTacToe.py:260:128: E501 line too long (129 > 127 characters)
./TicTacToe.py:356:128: E501 line too long (159 > 127 characters)

[codyswanner]

Errors straight from flake8:

./TicTacToe.py:94:128: E501 line too long (160 > 127 characters)

This is the one I didn't find. Ditto what I said about the player move prompt, we can clean this up or suppress the warning, no problem there.

I'm curious to check the lines that exceed the lower limits... but I'm working on pyTest and too lazy to go hunt down long lines, extend to your heart's content 😄

[MicahWW]

I will be setting the limit to 127, and fixing the lines over that. If we wish we can lower it in the future.