Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add option to define unique_id in yaml #416

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add option to define unique_id in yaml #416

wants to merge 2 commits into from

Conversation

Pave87
Copy link

@Pave87 Pave87 commented Oct 15, 2024

Added option for user to configure unique_id that is used as entity id.
This can help people who use Nordpool sensor in alot of places.

koopee
koopee reviewed Oct 16, 2024
View reviewed changes
Copy link

@koopee koopee left a comment •
edited
Loading

Choose a reason for hiding this comment

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

At least to me this looks very ok. I don't know if some tests should be added to this though.

Also the documentation would need updating to reflect this change as well.

koopee
koopee suggested changes Oct 16, 2024
View reviewed changes
Copy link

@koopee koopee left a comment

Choose a reason for hiding this comment

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

Documentation would need some changes as well.

@Pave87
Update README.md
3970008 
Documentation for overriding entity id
@Pave87
Copy link
Author

Pave87 commented Oct 16, 2024

Added some documentation of this to README.md

Will look into making tests for this but might take some as very new to Python and Home Assistant components.

koopee
koopee approved these changes Oct 24, 2024
View reviewed changes
Copy link

@koopee koopee left a comment

Choose a reason for hiding this comment

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

I did not manage to find testing for any other functionality for nordpool integration, so if there is no required testing for nordpool integration, I approve this.

@koopee
Copy link

koopee commented Nov 8, 2024

Why is this still pending? I would really much like this to be added to next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Repsionu Repsionu Repsionu approved these changes

@koopee koopee koopee approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Pave87 @koopee @Repsionu