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

Expose additional settings to vars config template for appservice-discord #835

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

skepticalwaves
Copy link
Contributor

No description provided.

@skepticalwaves
Copy link
Contributor Author

Does anything else need to be done for this to be merged? Is it of interest?

@spantaleev
Copy link
Owner

It looks good! I just wonder if we should do this or not. Exposing more and more settings.. Until when?

See this FAQ question - https://github.com/spantaleev/matrix-docker-ansible-deploy/blob/master/docs/faq.md#id-like-to-adjust-some-configuration-which-doesnt-have-a-corresponding-variable-how-do-i-do-it

It's especially problematic for Synapse and other services which have hundreds of potential settings we might wish to expose using variables. Having a dedicated variable for each one just makes the roles/matrix-XXX/defaults/main.yml file cluttered with things, which hides away the really important variables that the playbook does need to manage.

#796 is kind of a similar thing. My gut feeling is - we shouldn't do this. These settings aren't things that 99% of people would need. For the 1% that do need them, they have other (and simple at that) ways of achieving what they want. For appservice-discord, that's matrix_appservice_discord_configuration_extension_yaml.

@pushytoxin
Copy link
Contributor

The config extensions variables provide a powerful and amazing way to handle the fine details, preferences, fine tuning of homeserver internals.

Perhaps these lines, if important, should go into the commented-out example yaml section of the _configuration_extension_yaml variable, if they need to be highlighted.

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.

None yet

3 participants