Use setup.cfg to configure flake8, instead of in the ci code

[SubaruArai]

This makes flake8's configuration more discoverable and easier to use with other tools (IDE, etc.).

Regression: it seems that "max-complexity" wasn't properly enforced, now a lot of code is detected as too complex. (warn if > 10, max is 25)
We may need to adjust the value.
Complexity adjusted to 26 to satisfy the current code's needs.

[SubaruArai]

@cottsay What do you say? There are dozens of functions flagged as "too complex"(C901).

I suggest changing max-complexity to 25 or 30.

[SubaruArai]

We might as well revert #912 if this fixes those problems on macos. It might be related to complexity not being properly checked, due to using the old API.

[SubaruArai]

Just FYI, bumping flake8 to 6.1.0 added these errors:

• E721: do not compare types, for exact checks use is / is not, for instance checks use isinstance()
• E231: missing whitespace after ','

Personally, I think both are valid and should be fixed in a separate PR.

[cottsay]

If we're not enforcing the rule right now, the first move is to set a backstop at a higher level and get it merged before things get worse.

We can clean up the complexity and reduce the limit in a future change.

[SubaruArai]

@cottsay by backstop, do you mean changing the complexity in this pr from 10 to around (whatever smallest that works)?

[cottsay]

do you mean changing the complexity in this pr from 10 to around (whatever smallest that works)?

Yes

[SubaruArai]

@cottsay Done, it's ready to be merged!

[cottsay]

Thanks for tackling this. Only major concern is that we'd like to continue to support Python 2 until the next release.

Out of curiosity, why wasn't max_complexity enforced before? Underscore vs hyphen?

[SubaruArai]

Out of curiosity, why wasn't max_complexity enforced before? Underscore vs hyphen?

Looks like the legacy API accepted complexity when running check_file(). Maybe this was the reason?

[cottsay]

This is a deviation from how we've been doing flake8 tests in the rest of ROS, but the upstream churn has been breaking us constantly and I don't see any reason not to take this route everywhere else, too.

That said, I'd like to hear from other ros-infrastructure maintainers if they see any drawbacks with the process invocation approach taken here.

[nuclearsandwich]

That said, I'd like to hear from other ros-infrastructure maintainers if they see any drawbacks with the process invocation approach taken here.

I tested locally with both passing and non-passing result and in both cases the flake8 output was properly set.

This might even make it easier for me to add some local overrides (i.e. I have a .direnv folder in the repository that also needs to be excluded but haven't just caved and made it part of the file's default).