-
Notifications
You must be signed in to change notification settings - Fork 2.3k
[PAVST] Add units to constant name #41805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request improves code clarity by renaming a constant to include its units, which is a good practice. However, the change is incomplete as the usage of this constant was not updated in the allocate_one_pushav_transport method. This will cause a NameError at runtime. I've left a comment on the constant's definition pointing out what needs to be fixed.
|
PR #41805: Size comparison from 66389a9 to c590e52 Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Summary
To align with coding practices change the name of the constant in PushAVTestBase to be explicit on the units being represented (Seconds)
Related issues
Testing
Via CI
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See: Pull Request Guidelines