Skip to content

Build instances of the calendar providers before using them - #29231

Merged
ChristophWurst merged 1 commit into
masterfrom
fix/calendar-providers-instances
Oct 14, 2021
Merged

Build instances of the calendar providers before using them#29231
ChristophWurst merged 1 commit into
masterfrom
fix/calendar-providers-instances

Conversation

@ChristophWurst

Copy link
Copy Markdown
Member

What we get from the registration context are only the class names. We
still have to load the classes before we can use them.

Follow-up to #28970

@kesselbkesselb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

I think the calendar manager should have a method getCalendarProviders but might be yagni ;)

Comment threadlib/private/Calendar/Manager.php

@miaulalalamiaulalala left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ChristophWurstChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 14, 2021
@ChristophWurst

Copy link
Copy Markdown
MemberAuthor

LGTM

And tested, right? I don't have the setup to verify my changes actually work :)

@kesselb

kesselb commented Oct 14, 2021

Copy link
Copy Markdown
Contributor

I think the calendar manager should have a method getCalendarProviders but might be yagni ;)

We already have loadCalendars. Does it make sense to merge this calendar providers step into loadCalendars? Looks to complicated for now ;)

What we get from the registration context are only the class names. We
still have to load the classes before we can use them.
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst
ChristophWurstforce-pushed the fix/calendar-providers-instances branch from 707043b to c75a12cCompareOctober 14, 2021 14:07
@ChristophWurst
ChristophWurst merged commit 67baaaa into masterOct 14, 2021
@ChristophWurst
ChristophWurst deleted the fix/calendar-providers-instances branch October 14, 2021 17:29
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ChristophWurst@kesselb@miaulalala