search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.92k stars 6.65k forks source link

scripts: twister: Apply ruff to environment.py #81714

Open LukaszMrugala opened 1 day ago

LukaszMrugala commented 1 day ago

Ruff formatting applied to environment.py file. Linting problems detected by ruff fixed. As this majorly changes file lines, help strings were normalised as multiline strings. Parameters mentioned in help strings now are inside quotation marks. All quotation marks inside help strings changed to single quotes. Minor typos and grammar errors fixed.

Unit test changed to incorporate changes.

We didn't want to make a have commit fixing all ruff formatting, as it would be too big and unwieldy. (As I understand the discussion in the PR introducing ruff here) Instead, we wanted a gradual and incremental approach, where new files would adhere to ruff and old ones would get there >eventually<.

However, coupling such big formatting changes with logic changes can make the differences and bugs hard to spot in the PR, so it would be better to bring up the older files in separate PRs. Otherwise, we may never have compliant Twister files.

Concerning Twister, twisterlib is my main concern. The environment.py was deemed a good starting point. After those, I'd like to tackle the Twister test files.

The matter of PR sizes is an open question that I'd like to introduce here. Should there be a PR per file? One PR per directory group (e.g. pylib or twisterlib in this case)? Every file touched by the formatter can often be majorly changed, so I'd assume that the number of additions and deletions is a good guide.

pdgendt commented 1 day ago

For traceability I would advice to focus on the linter fixes and leave the formatting for now. The linter fixes are currently hidden and might have side-effects that need to be looked at in a review.

LukaszMrugala commented 1 day ago

For traceability I would advice to focus on the linter fixes and leave the formatting for now. The linter fixes are currently hidden and might have side-effects that need to be looked at in a review.

I'd wager that linter fixes could be bunched together, as the aren't as expansive as formatter-based changes. I didn't verify all files, but I might be able to bundle all twisterlib linter fixes in one PR.

pdgendt commented 1 day ago

I have done something similar for the west runners here https://github.com/zephyrproject-rtos/zephyr/pull/81667

LukaszMrugala commented 1 day ago

I have done something similar for the west runners here #81667

I'll maybe keep this one as a draft, so as to first have the fixes merged (via a separate PR akin to yours for west runners), and then do reformatting.