Skip to content

Return full list of events for broadcasting#2787

Merged
MichMich merged 2 commits intoMagicMirrorOrg:developfrom
kolbyjack:broadcast_all_events
Jan 19, 2022
Merged

Return full list of events for broadcasting#2787
MichMich merged 2 commits intoMagicMirrorOrg:developfrom
kolbyjack:broadcast_all_events

Conversation

@kolbyjack
Copy link
Contributor

I have several modules that provide alternate displays for calendar events, and currently, if I want the calendar module to return all of the future events for the current month (for my MMM-MonthlyCalendar module), then I need to configure maximumEvents to be large enough that the calendar module itself needs to be hidden. Instead, the configured display limits should only be imposed when generating the list of events to display, not when fetching events from configured calendars.

@MichMich
Copy link
Collaborator

I understand the need and it's fine to implement. But I'm not a big fan of the forDisplay attribute since it doesn't tell what will happen if it's set to true. Maybe rename it to something like limitNumberOfEntries?

@codecov-commenter
Copy link

Codecov Report

Merging #2787 (e3857ca) into develop (fcb0d33) will increase coverage by 0.37%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2787      +/-   ##
===========================================
+ Coverage    68.67%   69.05%   +0.37%     
===========================================
  Files            8        8              
  Lines          265      265              
===========================================
+ Hits           182      183       +1     
+ Misses          83       82       -1     
Impacted Files Coverage Δ
js/server.js 87.03% <0.00%> (+1.85%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcb0d33...e3857ca. Read the comment docs.

@khassel
Copy link
Collaborator

khassel commented Jan 17, 2022

@MichMich the test fail is unrelated to the code changes, same test ran locally on my machine and in all tests except node 14 (by now I think the vm running the tests was extremly slow but if this occurs again we have to increase timeout).

@MichMich
Copy link
Collaborator

I'll rerun the jobs to check if that helps.

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.

4 participants