-
Notifications
You must be signed in to change notification settings - Fork 421
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
Support dynamic domains in mod_offline #3164
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3164 +/- ##
==========================================
+ Coverage 77.41% 80.20% +2.78%
==========================================
Files 396 396
Lines 32306 32316 +10
==========================================
+ Hits 25010 25918 +908
+ Misses 7296 6398 -898
Continue to review full report at Codecov.
|
1af8fec
to
6de2566
Compare
Update the backend callbacks. The tests will not pass until the backend modules are also updated. Also: clean up comments and type specs.
- Add type specs to callbacks - Let the remove_*_messages functions remove only the messages for the provided LServer, not for all domains. This is done in a simple way, it can be optimized later if needed.
- Add type specs to callbacks - Let the remove_*_messages functions remove only the messages for the provided LServer, not for all domains. This is done in a simple way, it can be optimized later if needed.
Enable it conditionally in the config template. Also: - Rearrange newlines to make both the templates and the resulting files more readable. The only inconsitency is the indentation in case of multiple options. It's a corner case not occuring atm and it will not affect the default config visible to the end user - only the dev config for mim1. There will be a PR in the future which will unify the templating for all modules. - Do not set options with values same as the default ones.
Also: - Add specs - Simplify calling resend_offline_messages_hook
129ed2a
to
aad2505
Compare
{{#mod_cache_users}} | ||
[modules.mod_cache_users] | ||
{{{mod_cache_users}}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This disables this module in the prod config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is only a whitespace change, it cannot disable anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was disabled before and remains unchanged. Let's change this in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Main changes:
mod_offline
and its backends.Side changes:
remove_*_messages
to only remove messages for the domain that was provided. The backends were behaving inconsistently before (it worked as expected only for Riak).Not included in this PR:
mod_offline_chatmarkers
. This is an undocumented module which is disabled by default.ok
is returned. I didn't want to change too much and these issues don't seem to be critical. It's a bit out of scope as well.