Skip to content

Use strings instead of f-strings for constants#40619

Merged
frenck merged 6 commits intohome-assistant:devfrom
springstan:use-strings-instead-of-f-strings-for-constants
Nov 9, 2020
Merged

Use strings instead of f-strings for constants#40619
frenck merged 6 commits intohome-assistant:devfrom
springstan:use-strings-instead-of-f-strings-for-constants

Conversation

@springstan
Copy link
Copy Markdown
Member

Breaking change

Proposed change

After the discussion in #40436 (comment) I decided to try and remove as many f-strings from constants.
I have changed the following things and split them up via commits:

  • Use strings instead of f-strings for HA constants
  • Use string instead of f-string for accuweather
  • Add and use PRECIPITATION_MILLIMETERS_PER_HOUR constant
  • Add and use speed and volume flow rate units
  • Use irridation unit in ambient station

There are a lot more constant that use f-strings, but I wanted to correct the most used ones in the code base.

cc @frenck

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
n/a

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

Comment thread homeassistant/const.py Outdated
Comment thread homeassistant/const.py Outdated
@frenck
Copy link
Copy Markdown
Member

frenck commented Nov 8, 2020

@springstan Could you pick up the merge conflict? I've added this PR to my notification list for cleanup after it.

@springstan
Copy link
Copy Markdown
Member Author

Sure, solved the merge conflicts ✌️

Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @springstan 👍

@frenck frenck merged commit 30b9489 into home-assistant:dev Nov 9, 2020
@springstan springstan deleted the use-strings-instead-of-f-strings-for-constants branch November 9, 2020 09:33
shbatm added a commit to shbatm/hacs-isy994 that referenced this pull request Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants