-
-
Notifications
You must be signed in to change notification settings - Fork 288
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
Zigbee2mqtt : Remove containers when disabling integration #2109
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2109 +/- ##
=======================================
Coverage 98.41% 98.41%
=======================================
Files 865 865
Lines 14169 14171 +2
=======================================
+ Hits 13945 13947 +2
Misses 224 224 ☔ View full report in Codecov by Sentry. |
Deploying gladys-plus with Cloudflare Pages
|
#2675 Bundle Size — 10.18MiB (0%).
Warning Bundle contains 3 duplicate packages – View duplicate packages Bundle metrics
|
Current #2675 |
Baseline #2665 |
|
---|---|---|
Initial JS | 5.49MiB |
5.49MiB |
Initial CSS | 303.12KiB |
303.12KiB |
Cache Invalidation | 0% |
53.79% |
Chunks | 51 |
51 |
Assets | 171 |
171 |
Modules | 1490 |
1490 |
Duplicate Modules | 21 |
21 |
Duplicate Code | 0.82% |
0.82% |
Packages | 124 |
124 |
Duplicate Packages | 3 |
3 |
Bundle size by type no changes
Current #2675 |
Baseline #2665 |
|
---|---|---|
JS | 7.27MiB |
7.27MiB |
IMG | 2.48MiB |
2.48MiB |
CSS | 319.91KiB |
319.91KiB |
Fonts | 93.55KiB |
93.55KiB |
Other | 17.62KiB |
17.62KiB |
HTML | 13.58KiB |
13.58KiB |
Bundle analysis report Branch fix-zigbee2mqtt-re-configuration Project dashboard
@atrovato @cicoub13 Do you see any side effect on this PR ? I've tested it in real life, and it works great (it fixes the bugs we are seeing on the forum: https://community.gladysassistant.com/t/non-communication-zigbee2mqtt/9027/12?u=pierre-gilles) |
The PR is good and I see no side effect. It will be cleaner. Not sure it will solve the password issue or dongle configuration changed (use cases already managed in other parts of the code) |
Yes it solves the issues, because when re-installing the containers, it falls into another case and it re-installs everything from scratch (including configuration files). When the container is just stopped (not removed), the container is just restarted I've tried in real life, and it works with this version |
Pull Request check-list
To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:
npm test
on both front/server)npm run eslint
on both front/server)npm run prettier
on both front/server)npm run compare-translations
on front)Description of change
Fix #2108 - Remove containers when disabling the integration. It fixes a bug when the user disable then re-enable containers, the new credentials are not taken into account