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

Reply via email to