Add sendNotifications option to clock module #3059
Conversation
|
@rejas will you wait for the reaction of the original author or shall I merge? |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## develop #3059 +/- ##
===========================================
+ Coverage 27.52% 27.55% +0.03%
===========================================
Files 52 52
Lines 11568 11573 +5
===========================================
+ Hits 3184 3189 +5
Misses 8384 8384
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
I'd like to know your opnion on my question regarding the default value first |
|
I'm fine with this, the other way round the majority of users would still have the unwanted noise. May this should mentioned in the docs as a "breaking" change. |
|
I also think it makes sense to set the default to |
In the CHANGELOG or where do you mean? The docs website doesnt have any version information. |
I meant the docs, the new property |
Ok, will open a PR there "soonish". This PR is therefor ready to merge @khassel |
Fixes MagicMirrorOrg#3056 One question here would be if the default for this new option should be true or false. True: keeps the current behaviour, nobody needs to change his config if they rely on this option False: keeps the clock notifications quiet, doesnt waste time/resources, keeps the noise low Maybe the original author @cybex-dev can weigh in on this, and why he added this notification. --------- Co-authored-by: veeck <michael@veeck.de>
Fixes #3056
One question here would be if the default for this new option should be true or false.
True: keeps the current behaviour, nobody needs to change his config if they rely on this option
False: keeps the clock notifications quiet, doesnt waste time/resources, keeps the noise low
Maybe the original author @cybex-dev can weigh in on this, and why he added this notification.