search

sfPlayer1 / Matcher

Tool for tracking elements in obfuscated Java archives across releases
GNU General Public License v3.0
75 stars 41 forks source link

Use proper logging library #29

Closed NebelNidas closed 7 months ago

NebelNidas commented 1 year ago

Replaces all System.out calls with usages of a proper logger library. Initially I wanted to use SLF4J's API with Log4j as implementation, but I couldn't get it to work with the Java module system, so I just used Log4j's API as well. Now uses SLF4J as API and Tinylog as implementation. Also fixes some inconsistent capitalization of logged sentences.

liach commented 1 year ago

Instead of using apache with those unnecessary and bug-prone "features", how about we use System.Logger and System.getLogger instead, available since JDK 9?

NebelNidas commented 1 year ago

On second thought (and after seeing its JAR size), I agree that Log4j is indeed overkill for Matcher. I'm not a huge fan of System.Logger though, and since I want to split Matcher into a core (Java 8 based) and a gui (Java 17 based) module later down the road, using Java 9 features wouldn't work out anyway. I think I'm going to go with SLF4J and Tinylog for now

liach commented 1 year ago

On second thought (and after seeing its JAR size), I agree that Log4j is indeed overkill for Matcher. I'm not a huge fan of System.Logger though, and since I want to split Matcher into a core (Java 8 based) and a gui (Java 17 based) module later down the road, using Java 9 features wouldn't work out anyway. I think I'm going to go with SLF4J and Tinylog for now

Downgrading Matcher from Java 17 to 8, which already has premier support ended, isn't quite a good idea. https://github.com/sfPlayer1/Matcher/blob/f01ed5a7813458abb55a254625e3c4ad48c16590/build.gradle#L36-L45

Your code using matcher directly should migrate to Java 17. 8 is sweet for those sun package stuff and unrestricted reflection to change final fields, but you shouldn't have relied on them in the first place.

NebelNidas commented 1 year ago

I completely forgot that Matcher uses the Java module system already, so yeah, I should target Java 11 instead. I don't want to force library consumers into requiring Java 17 just yet, especially since Matcher's core functionality doesn't use any Java 12+ features anyway. Player can always bump it later if required.