search

byuccl / bfasst

Tools for FPGA Assurance Flows
Apache License 2.0
12 stars 4 forks source link

Verify pylint disable locations #363

Closed jgoeders closed 7 months ago

jgoeders commented 7 months ago

Given major refactors in #316 and #360 (once it's merged), we should go through and check the several locations where we have pylint: disable in the Flow and Tool code, as I suspect many of the locations could be removed.

KeenanRileyFaulkner commented 7 months ago

Our pylint min_similarity_lines is set to 4, I propose moving it to 5 because there were a couple of spots in the create_build_snippets and __init__ methods of our biggest flows where there were more than four tools being used, and it caused a duplication error without the disables. 5 lines would allow us to remove these disables.

Currently, I'm doing as asked above and only looking at the Flow and Tool code. The unittest scripts are full of duplicate code and the pylint disables, but I don't think that's a bad thing given that they are unit tests and we don't want to make them opaque. Also the rw utility has several other types of disables. Also, most of the old legacy flows have duplicate code disables, but I don't think we should worry about that either.

Thoughts? @jgoeders

jgoeders commented 7 months ago

I'm fine raising line similarity to something a bit larger.