On 8/21/2024 11:06 AM, Burakov, Anatoly wrote:
On 8/21/2024 10:52 AM, Burakov, Anatoly wrote:
On 8/20/2024 5:57 PM, Robin Jarry wrote:
Anatoly Burakov, Aug 20, 2024 at 17:35:
Update coding style:
- Make the code PEP-484 compliant
- Add more comments, improve readability, use f-strings everywhere
- Use quotes consistently
- Address all Python static analysis (e.g. mypy, pylint) warnings
- Improve error handling
- Refactor printing and sysfs/procfs access functions
- Sort output by NUMA node
Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
---
Notes:
v1 -> v2:
- Added commit that sorted output by NUMA node
v2 -> v3:
- Rewrite of the script as suggested by reviewers
Instead of debating about coding style. I'd like to enforce
black/ruff for new scripts and/or rewrites.
The code looks good to me, but could you pass through one of these
tools and send a v4?
black usertools/dpdk-hugepages.py
or
ruff format usertools/dpdk-hugepages.py
I think they output the exact same code formatting but I could be wrong.
Hi,
My IDE is already set up to auto-format with Ruff since our last
conversation, so this is already formatted. I ran ruff format command
just in case but it produced no changes.
So, no v4 necessary unless you think there are any changes to be made
about the code :)
Actually, I take that back - I had a configuration mishap and didn't
notice that I wasn't using Ruff for formatting on the machine I was
creating the commits.
Still, cpu_layout's formatting is not affected, but hugepage script is.
However, after formatting with ruff, I can see that 1) most single
quotes became double quotes, 2) some lines I broke up for readability,
are no longer broken up, and 3) some lines I broke up to avoid exceeding
the 80 symbols count, are no longer broken up.
I'll see if using Black yields different results.
Regarding line length, it seems that it's configurable. Perhaps we could
include a Ruff/Black configuration file with DPDK to solve this problem
once and for all? Adding --line-length=79 to ruff config addresses the
last issue, but it wouldn't be necessary if there was a Ruff
configuration file in the repo. I can live with first two things that I
highlighted.
--
Thanks,
Anatoly