-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
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
Improve reading clarity of steps code in scripts helper #134395
Improve reading clarity of steps code in scripts helper #134395
Conversation
b48acf1
to
a5ed0a7
Compare
de9c9b3
to
a0e0980
Compare
a0e0980
to
fa5f644
Compare
This probably is better/nicer; it's just very difficult to review in either split or unified diff format; or by viewing the interim steps (moving one or two chunks at a time). |
For reviewing I'd suggest clicking on |
I agree that is difficult to see what has changed. It would be nicer to do this in a few smaller merges and only rearrange the location of one function per merge so the changes can easily be seen. |
I think splitting is a good idea, but going only one function per PR would be overkill. Here's part 1: #138628 |
The advantage to one per function is I can review it on my mobile and check it in my head vs waiting to have a time chunk at my desk which is much more of a premium. Either way we can get it merged eventually, it will take longer to review if I can't do it mobile |
I checked each relocation manually, and merged that one so hopefully this one will be a bit smaller once rebased |
d59f19b
to
75a4672
Compare
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.
Looks good. I've checked each moved block manually.
Proposed change
Improve reading clarity of steps code in scripts helper.
No significant code changes, just reorganizing code to group it more logically and make it easier to read.
The only changes made are:
async_step_
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: