Skip to content
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

Don't resolve services when binding listeners #1700

Closed
wants to merge 1 commit into from

Conversation

franzliedke
Copy link
Contributor

Next part of solving flarum/issue-archive#269

Changes proposed in this pull request:
Replace event subscribers with event listeners in core, where it makes sense.

Exceptions:

  • Subscribers without constructor dependencies - These have no cost of dependencies, and are useful for grouping a bunch of listeners that simply operate on models.
    • Flarum\User\UserMetadataUpdater
    • Flarum\Discussion\DiscussionMetadataUpdater
  • Policy subclasses - These are a bit more complex, as a base class and lots of logic is involved - haven't thought about the best way to get rid of the subscriber model here.

Confirmed

  • Backend changes: tests are green (run php vendor/bin/phpunit).

@franzliedke
Copy link
Contributor Author

I pushed these (and then some) directly on master. Have gathered enough experience doing all of that for all the extensions. 😉

@franzliedke franzliedke deleted the fl/1578-remove-subscribers branch December 19, 2018 21:49
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.

1 participant