Skip to content

Validate UUID for tankerkoenig#32805

Merged
bdraco merged 2 commits intohome-assistant:devfrom
guillempages:validate_uuid
Mar 21, 2020
Merged

Validate UUID for tankerkoenig#32805
bdraco merged 2 commits intohome-assistant:devfrom
guillempages:validate_uuid

Conversation

@guillempages
Copy link
Copy Markdown
Contributor

@guillempages guillempages commented Mar 14, 2020

Breaking change

Proposed change

Add custom validators for the UUIDs used in the tankerkoenig integration, instead of validating against generic strings.

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
tankerkoenig:
  api_key: YOUR_API_KEY
  radius: 1
  fuel_type:
    - "diesel"

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

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

@guillempages
Validate UUIDs against custom validator
73539d3 
Instead of just validating against strings, use a custom validator,
so that the format can be checked.
bdraco
bdraco requested changes Mar 15, 2020
View reviewed changes
Comment thread homeassistant/components/tankerkoenig/__init__.py
@guillempages guillempages requested a review from bdraco March 21, 2020 13:34
@bdraco bdraco merged commit 49c2a4a into home-assistant:dev Mar 21, 2020
MartinHjelmare
MartinHjelmare reviewed Mar 21, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Please address the comment in a new PR.

Comment thread tests/components/tankerkoenig/test_tankerkoenig_validators.py
from homeassistant.components.tankerkoenig import uuid4_string


class TestUUID4StringValidator(unittest.TestCase):
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Mar 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

All new tests must be standalone pytest test functions.

Copy link
Copy Markdown
Contributor Author

@guillempages guillempages Mar 21, 2020

Choose a reason for hiding this comment

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

Done in #33115

@guillempages guillempages deleted the validate_uuid branch March 21, 2020 19:37
This was referenced Mar 21, 2020
Merged
Closed
@balloob balloob added this to the 0.107.5 milestone Mar 21, 2020
balloob pushed a commit that referenced this pull request Mar 21, 2020
@guillempages @balloob
Validate UUID for tankerkoenig (#32805)
1015ce1 
* Validate UUIDs against custom validator

Instead of just validating against strings, use a custom validator,
so that the format can be checked.

* Add tests for custom UUID4 validator
@balloob balloob mentioned this pull request Mar 21, 2020
Merged
@guillempages guillempages mentioned this pull request Mar 21, 2020
Closed
20 tasks
@balloob balloob removed this from the 0.107.5 milestone Mar 21, 2020
guillempages added a commit to guillempages/home-assistant that referenced this pull request Mar 22, 2020
@guillempages
Revert "Validate UUID for tankerkoenig (home-assistant#32805)"
0ad2273 
This reverts commit 49c2a4a.
The validation was causing more problems than it was fixing, so removed
it completely
@guillempages guillempages mentioned this pull request Mar 22, 2020
Merged
20 tasks
balloob pushed a commit that referenced this pull request Mar 22, 2020
@guillempages
Revert "Validate UUID for tankerkoenig (#32805)" (#33123)
99d732b 
This reverts commit 49c2a4a.
The validation was causing more problems than it was fixing, so removed
it completely
@lock lock Bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@MartinHjelmare MartinHjelmare MartinHjelmare left review comments

@bdraco bdraco bdraco approved these changes

Assignees

No one assigned

Labels

by-code-owner cla-signed has-tests integration: tankerkoenig small-pr PRs with less than 30 lines.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@guillempages @bdraco @MartinHjelmare @balloob @homeassistant