On Mon, May 9, 2022 at 11:22 PM David Gow davidgow@google.com wrote:
On Tue, May 10, 2022 at 4:49 AM Daniel Latypov dlatypov@google.com wrote:
This primarily comes from running pylint over kunit tool code and ignoring some warnings we don't care about. If we ever got a fully clean setup, we could add this to run_checks.py, but we're not there yet.
Fix things like
- Drop unused imports
- check `is None`, not `== None` (see PEP 8)
- remove redundant parens around returns
- remove redundant `else` / convert `elif` to `if` where appropriate
Personally, I find the explicit 'elif' much more readable in most of these cases, but if we're annoying a linter, I guess we should change them...
Same, some of them felt a tad more readable, but using `if` is a bit more explicit about the actual control flow. For short branches, like most of the ones here, they don't make too much of a difference, but for longer blocks of code, this can help.
E.g. if one sees elif check2(): do_thing2() one might think they can tack on a do_cleanup() and have it run for all branches, if they're not careful.
- rename make_arch_qemuconfig() param to base_kunitconfig (this is the name used in the subclass, and it's a better one)
- kunit_tool_test: check the exit code for SystemExit (could be 0)
Signed-off-by: Daniel Latypov dlatypov@google.com
All of these changes seem correct to me, even if I'm not sure I'd bother with most of them if they weren't causing pylint to show errors.
Given that apparently it does, though, I'm okay with it going through. (I'll just grumble quietly in my corner. :-))
To be fair, we could ignore more of these warnings. If we ever try to get the code clean wrt linter warnings, we'd already need to tweak the settings.
But I think I've taken care of most of the reasonable warnings in this patch.