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

Standardize user mention formatting #289

Open
ghost opened this issue Sep 13, 2021 · 4 comments
Open

Standardize user mention formatting #289

ghost opened this issue Sep 13, 2021 · 4 comments

Comments

@ghost
Copy link

ghost commented Sep 13, 2021

According to LogsPlugin.ts#L123, ping_user is deprecated, implying that allow_user_mentions is the preferred option. Yet in getLogMessage.ts#L92, the user mention formatting result used in logs is determined by both ping_user and allow_user_mentions.

If ping_user was internally marked as deprecated, why is it still being used to determine a user mention formatting result?
Instead, could we just standardize allow_user_mentions as the determining condition for whether or not verboseUserMention() is called instead of verboseUserName()?

if (config.allow_user_mentions) {
  mentions.push(verboseUserMention(user));
} else {
  mentions.push(verboseUserName(user));
}
@xPolar
Copy link

xPolar commented Oct 6, 2021

I'm assuming ping_user is still in use even though it's deprecated for backwards compatibility and etc. I'd say there are still quite a few servers that use ping_user not knowing it's been deprecated, I'd say the Dragory and the other developers wouldn't want to make a breaking change such as that.

@ghost
Copy link
Author

ghost commented Oct 7, 2021

I wouldn't doubt that it's still used. However, it can be a bit misleading and confusing if you haven't inspected the source code, that's all I'm saying. I'm seeking standardization. Furthermore, we shouldn't be afraid of removing deprecations. It's not really a "breaking change" either. The most you'll get are a few logs where the user is mentioned. Dragory can post an announcement as well, if required.

@Dragory
Copy link
Collaborator

Dragory commented Oct 17, 2021

Currently the main blocker for removing deprecations is the lack of a robust way to programmatically edit and migrate server configs (to remove/rename/etc. the deprecated options from them). There are some solutions I'm looking into; the main obstacle is preserving YAML formatting and comments. The yawn-yaml package looks very promising in that area.

@ghost
Copy link
Author

ghost commented Oct 17, 2021

Yeah, that's usually a problem with removing deprecations. Maybe you could officially announce it as deprecated along with plans to remove it with a couple months and suggest the alternative solution?

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

No branches or pull requests

2 participants