Skip to content

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
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