Re: [PATCH 1/9] scripts/performance: Refactor topN_perf.py

[John Snow]

On 10/1/20 4:41 PM, John Snow wrote:
> I realize this review comes well after you are no longer being paid to 
> work on this, so I am offering my time to help polish your patches if 
> you would like.

Actually, I see now that you are adding your name to the MAINTAINERS 
file here, so I suspect you probably rather want to be more involved 
than not.

I cleaned up patch 1/9 provisionally with my own style preferences, but 
it's all just style stuff, and it's mostly things I wouldn't actually 
require you to do (...I went way overboard.)

https://gitlab.com/jsnow/qemu/-/commit/c66a4a6ca8ccc3d406b92796935f92057bf1e48d


What I'd recommend for your cleanup is actually *much* simpler;

Use pylint 2.6.0 and flake8 3.8.3:

> pip3 install --user pylint==2.6.0 flake8==3.8.3

flake8's default settings should be pretty good, but pylint has a lot of 
warnings you can ignore.

In particular, it's OK to use script-style python (Scripts with a 
#!/usr/bin/env python3, and where you do not use python functions to 
avoid side-effects that occur on 'import'.) In this case, IGNORE any of 
pylint's warnings telling you that you have too many lines, that you 
need to UPPERCASE variable names, etc. It just hurts readability here.

So I'd actually ask that you revise these patches to remove all of the 
UPPERCASE variable names, and then check your code with these:

flake8 topN_perf.py
pylint --disable=invalid-name topN_perf.py

Use your best judgment -- If something seems like it looks worse, it 
probably is. If in doubt, please reach out and ask.

--js