GlowstoneMC / Glowstone

A fast, customizable and compatible open source server for Minecraft: Java Edition
https://glowstone.net
Other
1.9k stars 273 forks source link

Separate message names from logging levels #909

Closed Pr0methean closed 6 years ago

Pr0methean commented 6 years ago

This removes ".warn.", ".error." and ".info." from the names of logging messages in strings.properties, for several reasons:

  1. Console messages about the same subject matter now appear together, even when they are split across multiple logging levels.
  2. Calls to the LoggableLocalizedStringImpl constructor already specify the logging level, so embedding it in the name violates the Don't Repeat Yourself and Separation of Concerns principles.
  3. If we ever need to change the logging level of a message once multiple locales exist, we then have to update it in all versions of the resource file, and we make translators more susceptible to merge conflicts.
  4. We may eventually want to use the same message in multiple situations, at different logging levels.

This PR does not change the hierarchy inside LocalizedStrings, for two reasons:

  1. The current scheme makes it clear what level we're logging at, without having to jump from the calling class to LocalizedStrings. Thus, while it still violates DRY and SoC, in that case it may be a price worth paying for the readability gain.
  2. If we do start using the same message at multiple logging levels, then each level will need a different LoggableLocalizedString instance, and we'll need to disambiguate between them. The current scheme accomplishes that.