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

Config fixes #432

Merged
merged 3 commits into from
Nov 23, 2022
Merged

Config fixes #432

merged 3 commits into from
Nov 23, 2022

Conversation

Gnuxie
Copy link
Contributor

@Gnuxie Gnuxie commented Nov 22, 2022

Use the npm package config to load the config

This is what was used prior to #347.
It was a nice idea motivated to drop a dependency that was confusing.
It was just never followed through and was underestimated how much disruption it would cause.
It was also believed that the library would mean there could only ever be one global copy of the config,
It was followed up by:
#369
#357
#429
#397
#365

For simplicity sake I am reinstating the library.
The practice of loading default.yaml by default is also dangerous
and has led to issues multiple times in #mjolnir:matrix.org.
It is a sample and not a default.

Add cli argument for the config

--mjolnir-config can be specified to give a path to the config.
The reason it uses mjolnir as a prefix is so it cannot be confused with the appservice arguments.
The documentation has been updated accordingly so that people use that instead of relying on the implicit environment variable behaviour.

This is what was used prior to #347.
It was a nice idea motivated to drop a dependency that was confusing.
It was just never followed through and was underestimated how much disruption it would cause.
It was also believed that the library would mean there could only ever be one global copy of the config,
It was followed up by:
#369
#357
#429
https://github.com/matrix-org/mjolnir/pull/397/files
#365

For simplicity sake I am reinstating the library.
The practice of loading default.yaml by default is also dangerous
and has led to issues multiple times in #mjolnir:matrix.org.
It is a sample and not a default.

In a following commit I will be adding the ability to specify the
config to use from the cli.
@Gnuxie Gnuxie requested a review from Yoric November 22, 2022 15:33
@Gnuxie Gnuxie force-pushed the gnuxie/config-fixes branch from d8469c6 to 56957c6 Compare November 22, 2022 15:43
@Gnuxie Gnuxie merged commit 333c55e into main Nov 23, 2022
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