-
-
Notifications
You must be signed in to change notification settings - Fork 29.3k
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
Add config flow to Meteo-France #29927
Add config flow to Meteo-France #29927
Conversation
Hey there @victorcerutti, @oncleben31, mind taking a look at this pull request as its been labeled with a integration ( |
8d1b15d
to
96a78be
Compare
96a78be
to
28f3f9d
Compare
e1daff0
to
ed718c1
Compare
@springstan if you have a little time, I will appreciated a review, and why not an approval 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Quentame I have found some time to look over your code, please see my review comments for more information 😊
Thanks a lot for taking your time on my PR and for your review @springstan |
Of course @Quentame, no problem at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
157a7f1
to
e1610df
Compare
e1610df
to
bd7ffb4
Compare
update=False, | ||
) as service_mock: | ||
service_mock.return_value.get_data.return_value = {"name": CITY_2_NAME} | ||
yield service_mock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MartinHjelmare Is it possible to make conditional tests with MagicMock ?
That would be like :
if service_mock.return_value.postal_code == CITY_1_POSTAL:
service_mock.return_value.get_data.return_value = {"name": CITY_1_NAME}
else if service_mock.return_value.postal_code == CITY_2_POSTAL:
service_mock.return_value.get_data.return_value = {"name": CITY_2_NAME}
Looked but can't find.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on what the attributes on the mock return. By default they return mocks, so they need to be set or specced by something.
Make sure to update the breaking change paragraph. Ping me when that is done and the tests are ready and I'll have a final look. |
@MartinHjelmare Documentation updated + breaking change paragraph updated + tests passing 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just some small comments.
Tested with import : meteo_france:
- city: Paris
- city: '75001'
- city: '75004'
- city: '75009'
- city: Lyon
- city: '69001'
- city: '69004'
- city: '69009'
- city: '74220' And needed a little more fix. |
Codecov Report
@@ Coverage Diff @@
## dev #29927 +/- ##
========================================
Coverage 94.62% 94.62%
========================================
Files 749 750 +1
Lines 54193 54313 +120
========================================
+ Hits 51278 51393 +115
- Misses 2915 2920 +5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Breaking Change:
No more
monitored_conditions
, all sensors will be added --> ADR-03Description:
Adding config flow to Météo-France
Pull request with documentation for home-assistant.io : home-assistant/home-assistant.io#11474
Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest
.requirements_all.txt
by runningpython3 -m script.gen_requirements_all
..coveragerc
.If the code does not interact with devices: