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

Make namespace configurable and improve config&logger handling #43

Merged
merged 5 commits into from
Mar 24, 2024

Conversation

Phoenix616
Copy link
Contributor

This adds the ability to pass the namespace that should be used for discovering servers in. (The default setting of an empty/null namespace should still behave like before and discover in any namespaces)

I also expanded the config handling to allow defining of the options via environment variables as well as loading of the config file from the plugin's DataDirectory directly (this avoids potential issues in environments where / is not the path separator or where the DataDirectory is not the default one)

Additionally to that I moved the Logger from JUL to the Velocity-devs suggested slf4j one (and directly included stacktraces in the errors). That results in better compatibility with certain environments as the JUL to slf4j translation which Velocity ships as a fallback can break in some cases. (E.g. when running inside containers it can log as ERROR instead of at the proper level)

Copy link
Member

@acrylic-style acrylic-style left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR!
After a quick look, I've found a potential issue that may affect the existing installation. (I have not run the tests yet so please correct me if I am wrong)
I will update README-JP.md later so don't worry about translation.

This should do nothing on file systems which ignore the case
Copy link
Member

@acrylic-style acrylic-style left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, merging!

@acrylic-style acrylic-style merged commit babf766 into AzisabaNetwork:main Mar 24, 2024
1 check passed
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