-
Notifications
You must be signed in to change notification settings - Fork 428
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
Toml/doc Operation and Maintenance update #2899
Conversation
doc/operation-and-maintenance/Cluster-configuration-and-node-management.md
Outdated
Show resolved
Hide resolved
doc/operation-and-maintenance/Cluster-management-considerations.md
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1a0ae0c
to
8b9c733
Compare
8620.1 / Erlang 23.0.3 / small_tests / da7d2ad 8620.2 / Erlang 23.0.3 / internal_mnesia / da7d2ad 8620.3 / Erlang 23.0.3 / odbc_mssql_mnesia / da7d2ad 8620.4 / Erlang 23.0.3 / mysql_redis / da7d2ad 8620.7 / Erlang 23.0.3 / elasticsearch_and_cassandra_mnesia / da7d2ad 8620.6 / Erlang 23.0.3 / ldap_mnesia / da7d2ad 8620.5 / Erlang 23.0.3 / riak_mnesia / da7d2ad 8620.9 / Erlang 22.3 / pgsql_mnesia / da7d2ad |
Codecov Report
@@ Coverage Diff @@
## master #2899 +/- ##
==========================================
+ Coverage 79.14% 79.16% +0.01%
==========================================
Files 379 379
Lines 32752 32752
==========================================
+ Hits 25923 25927 +4
+ Misses 6829 6825 -4
Continue to review full report at Codecov.
|
doc/operation-and-maintenance/Reloading-configuration-on-a-running-system.md
Show resolved
Hide resolved
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.
Just one comment, otherwise fantastic, thanks for the work! 👍
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.
A lot of really good changes here. I just have a few minor questions.
doc/operation-and-maintenance/Cluster-configuration-and-node-management.md
Show resolved
Hide resolved
doc/operation-and-maintenance/Cluster-configuration-and-node-management.md
Outdated
Show resolved
Hide resolved
doc/operation-and-maintenance/Reloading-configuration-on-a-running-system.md
Show resolved
Hide resolved
|
||
[[modules.mod_muc_light.config_schema]] | ||
field = "notification_sound" | ||
value = "" | ||
``` |
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.
Just an alternative, see which looks better:
config_schema = [
{field = "roomname", value = "The room"},
{field = "subject", value = "Chit-chat"},
{field = "background", value = ""},
{field = "notification_sound", value = ""}
]
Btw, how do we eliminate default_config
? it seems to be a config option created internally by muc_light, I wonder why it was in the example then.
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.
Thanks for the suggestion, it looks better I think, but I've made a big mistake. From what I'm reading now, the whole section doesn't make any sense as described now - without the default_config
option (as is in MIM from 3.6) the problem described in this paragraph will not appear. So it's completely senseless to describe it with TOML config as this problem only appears in older MIMs. I will revert changes to this file because of that.
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.
I think it's correct now that it's reverted.
Co-authored-by: K. Kraus <[email protected]>
Co-authored-by: Nelson Vides <[email protected]>
Co-authored-by: Paweł Chrząszcz <[email protected]>
ab74832
to
ce776b6
Compare
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.
I think it's all good now.
This PR updates the Operation and Maintenance part of the documentation to reflect the TOML configuration. It also contains many general fixes, like typos, grammar, etc (sometimes in files located outside the O&A part of the docs).
I did not touch the
Reloading-configuration-on-a-running-system.md
file as I'm not sure what to do with it. It seems to not really be reflected in the new TOML config (e.g. nonode_specific_options
in the TOML config). I hesitated from adding something like "This functionality is only available if you are using the older,cfg
config format" but this may be the best way to deal with this issue for now. I'd be happy to hear reviewers opinions on this topic.