Correctcurrent_temp local variable name in homekit _get_current_temperature#157202
Conversation
|
Hey there @bdraco, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
Would you please cleanup the PR summary. The template is a bit mangled |
|
What issue are you seeing? |
|
Ope! @bdraco In another browser I saw that there was one list under "If the code communicates with devices" that got 2-line newlines rather than 1-line, fixed now. Apologies for the extra whitespace 😳. 🥺 Github markdown thought that follow up item was part of the previous list |
current_temp local variablecurrent_temp local variable name in homekit _get_current_temperature
|
@bdraco 🥳🙏 Thank you so much! I think HA owns the template and developer docs, right? If that's true, are you If the later, is there anything from the notes that I might help with or a good place to move them for tracking? If the former, is there a good way/place to get the later's 👀 on the contributing issues experienced. I would love to (meta) contribute to the contributing ease/speed/experience! |
🙏 Thank you maintainers for your time and commitment to FOSS! 💖
Proposed change
Typo in
_get_current_temperature. Relevance: I was looking for the current temp and my learned workflow made it difficult to discover due to the misnomer. Just updating the copy-paste from the method above to match the intent. Non-functional change.It looks like copy-paste from _get_target_temperature Name chosen to pattern-match _get_
target_temperature.target_temp-> _get_current_temperature.current_temp👉 NOTE ⏱️Following this template accurately and in earnest added 2 hours to a 1 minute change. Most of that due to test latency & doing PRs, even with those being done in parallel - no open PRs were as trivial as this one. Of course, no change should be assumed to be 1 minute, but the 2 hours did seem in reducible excess. Happy and eager to help reduce that wherever there's appetite and interest!
Type of change
Additional information
@`maintainers: Should the PR template for this section direct(/permit, given the text at the very top ["DO NOT DELETE ANY TEXT from this template! (unless instructed)"]) us to remove those items in this list which are not applicable rather than leaving them empty (re "Please be sure to fill out additional details, if applicable")
Checklist
pre-commit run --all-fileshad many errors, none seemed related to this change but due to the volume it was difficult to telluv pip install -r requirements_test_all.txt-> "warning: The packagetyper==0.12.5does not have an extra namedall"ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
Documentation added/updated for www.home-assistant.io
"If user exposed functionality" -> ❌
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:
I have reviewed two other open pull requests in this repository.
❓ "two"? But the comment above says one!
Remove the restriction that Bluetooth login to the Switchbot account is only possible in active mode #157154 (review)
Add steam temperature number to lamarzocco #157167 (review)