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.

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

Moved INFO_RESOURCES access to instance get method #47

Merged
merged 5 commits into from
Mar 8, 2025

Conversation

danharvey
Copy link
Contributor

In pydaikin/daikin_brp069.py INFO_RESOURCES is modified on init but meant to be a constant. If init is called multiple times this will cause the list of resources to grow.

I've split out access of info resources to an instance method to allow modification per instance based on state.

This fixes #46

@fredrike
Copy link
Owner

Thanks for the pr!

Please make sure all tests passes and I'll be happy to merge this.

@fredrike
Copy link
Owner

fredrike commented Jan 10, 2025

This looks better than #44 as we should not modify the constant. I do however like the smaller change in #44.

@fredrike
Copy link
Owner

Please rebase and make sure that all tests complete.

@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2025

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 55.05%. Comparing base (2faa3a1) to head (0ee445c).
Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
pydaikin/daikin_base.py 33.33% 2 Missing ⚠️
pydaikin/daikin_brp069.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #47      +/-   ##
==========================================
- Coverage   55.32%   55.05%   -0.27%     
==========================================
  Files          11       11              
  Lines         817      821       +4     
==========================================
  Hits          452      452              
- Misses        365      369       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fredrike fredrike merged commit 48d249b into fredrike:master Mar 8, 2025
7 checks passed
@danharvey
Copy link
Contributor Author

@fredrike - thanks for fixing and landing this! sorry I wasn't able to get to this the past few months.

@danharvey danharvey deleted the fix-constant-modification branch March 10, 2025 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

daikin_brp069 modifies class constant on init
3 participants