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

Resolve SettingsRepositoryInterface as late as possible #5

Merged

Conversation

clarkwinkelmann
Copy link
Contributor

This gives the opportunity to other extensions to change its implementation
Also added the missing check for ForumSerializer so that the value only gets added to the forum payload and not every other serialized payload

This change would allow extenders like the one I provide in https://github.com/clarkwinkelmann/flarum-local-extenders#override-settings to override the setting value.

Let me know if you would have preferred I keep the original file formatting. My PhpStorm automatically formatted the file according to PSR-2.

This second PR uses the solution @franzliedke recommended in #4

This gives the opportunity to other extensions to change its implementation
Also added the missing check for ForumSerializer so that the value only gets added to the forum payload and not every other serialized payload
@therealsujitk
Copy link
Owner

therealsujitk commented Apr 26, 2020

@clarkwinkelmann thanks! I cannot test this right now or anytime soon because of several reasons but I will merge this pull request anyway because of your reputation with Flarum.

@therealsujitk therealsujitk merged commit cf35843 into therealsujitk:master Apr 26, 2020
@clarkwinkelmann
Copy link
Contributor Author

I did test the PR locally before submitting it 👍

@therealsujitk therealsujitk removed the enhancement New feature or request label Sep 18, 2021
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.

2 participants