Use Logger instead of println #126

2021-08-25T15:19:22Z

StoneLabs commented

2021-08-25 15:19:22 +00:00

(Migrated from github.com)

PR for #125.

👍 3 😕 1

StoneLabs commented

2021-09-09 14:43:52 +00:00

(Migrated from github.com)

@modmuss50, could you merge or close this if you find the time?
Thanks

@modmuss50, could you merge or close this if you find the time? Thanks

modmuss50 commented

2021-09-09 14:56:07 +00:00

(Migrated from github.com)

Does this really add any benefit? I know its more correct, but is there the worry that this will add extra confusion? Everyone getting started will know about printLn, but a logger is added complexity that wont help make people less confused.

Not sure what you guys think.

Does this really add any benefit? I know its more correct, but is there the worry that this will add extra confusion? Everyone getting started will know about printLn, but a logger is added complexity that wont help make people less confused. Not sure what you guys think.

👎 2

StoneLabs commented

2021-09-09 15:06:54 +00:00

(Migrated from github.com)

I believe if people know println they would also be familiar with the concept of a logger.

If they don't know either then they'll probably stick to the one used in the template. In that case, suggesting they use the logger might be a good idea.

I believe if people know println they would also be familiar with the concept of a logger. If they don't know either then they'll probably stick to the one used in the template. In that case, suggesting they use the logger might be a good idea.

👍 4

Jamalam360 commented

2021-09-09 16:18:08 +00:00

(Migrated from github.com)

I agree with what was said above. They will stick to what the example uses, so we can push them in the right direction standards wise by using a logger

Edit: start as you mean to go on is probably the expression I was looking for

I agree with what was said above. They will stick to what the example uses, so we can push them in the right direction standards wise by using a logger Edit: start as you mean to go on is probably the expression I was looking for

👍 4

StoneLabs commented

2021-09-21 11:56:49 +00:00

(Migrated from github.com)

@modmuss50 would you reconsider now that some others have weighed in?

modmuss50 (Migrated from github.com) reviewed 2021-09-21 11:58:07 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

modmuss50 (Migrated from github.com) commented

2021-09-21 11:58:06 +00:00

Why not LOGGER.info("Hello Fabric world!")?

Why not `LOGGER.info("Hello Fabric world!")`?

StoneLabs (Migrated from github.com) reviewed 2021-09-21 11:59:05 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

StoneLabs (Migrated from github.com) commented

2021-09-21 11:59:05 +00:00

Would be fine with me, i just thought this would more obviously hint towards other options besides INFO.

Should i change it?

Would be fine with me, i just thought this would more obviously hint towards other options besides INFO. Should i change it?

modmuss50 (Migrated from github.com) reviewed 2021-09-21 12:00:57 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

modmuss50 (Migrated from github.com) commented

2021-09-21 12:00:57 +00:00

I would, I generally dont pass the level like that in a case like this.

StoneLabs (Migrated from github.com) reviewed 2021-09-21 12:02:05 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

StoneLabs (Migrated from github.com) commented

2021-09-21 12:02:05 +00:00

Alright, give me a minute.

StoneLabs commented

2021-09-21 12:07:04 +00:00

(Migrated from github.com)

@modmuss50 I've changed the calls as requested.

modmuss50 (Migrated from github.com) approved these changes 2021-09-21 12:11:57 +00:00

Technici4n (Migrated from github.com) reviewed 2021-09-21 12:36:52 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

Technici4n (Migrated from github.com) commented

2021-09-21 12:36:52 +00:00

Might want to give the logger a name, for example LogManager.getLogger("ExampleMod").

Might want to give the logger a name, for example `LogManager.getLogger("ExampleMod")`.

StoneLabs (Migrated from github.com) reviewed 2021-09-21 12:45:09 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

StoneLabs (Migrated from github.com) commented

2021-09-21 12:45:09 +00:00

By default, the getLogger will use the Caller's class name. So that would be ExampleMod anyways.

// Returns a Logger with the name of the calling class.
// Returns:
//     The Logger for the calling class.
// Throws:
//     UnsupportedOperationException – if the calling class cannot be determined.
public static Logger getLogger() {
    return getLogger(StackLocatorUtil.getCallerClass(2));
}

By default, the getLogger will use the Caller's class name. So that would be ExampleMod anyways. ```java // Returns a Logger with the name of the calling class. // Returns: // The Logger for the calling class. // Throws: // UnsupportedOperationException – if the calling class cannot be determined. public static Logger getLogger() { return getLogger(StackLocatorUtil.getCallerClass(2)); } ```

liach (Migrated from github.com) reviewed 2021-09-21 14:29:07 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

liach (Migrated from github.com) commented

2021-09-21 14:29:07 +00:00

Imo we should use the mod id for the logger name.

sfPlayer1 (Migrated from github.com) reviewed 2021-09-21 19:35:30 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

sfPlayer1 (Migrated from github.com) commented

2021-09-21 19:35:29 +00:00

Yes, definitely mod id. It is the most useful for the end user and we want to promote best practices.

StoneLabs (Migrated from github.com) reviewed 2021-09-21 21:33:16 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

StoneLabs (Migrated from github.com) commented

2021-09-21 21:33:16 +00:00

@sfPlayer1 How would I get the mod id? Or do you mean LogManager.getLogger("modid")?

@sfPlayer1 How would I get the mod id? Or do you mean `LogManager.getLogger("modid")`?

sfPlayer1 (Migrated from github.com) reviewed 2021-09-21 21:38:17 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

sfPlayer1 (Migrated from github.com) commented

2021-09-21 21:38:17 +00:00

Yes, but please add a comment to encourage using the actual mod id. I may add something to Loader to determine it from a class eventually, but for now it's not really supported.

StoneLabs (Migrated from github.com) reviewed 2021-09-21 21:47:40 +00:00

src/main/java/net/fabricmc/example/ExampleMod.java

StoneLabs (Migrated from github.com) commented

2021-09-21 21:47:40 +00:00

How does this sound?

// This logger is used to write text to the console and the log file.
// It is considered best practice to use your mod id as the logger's name.
// That way, it's clear which mod wrote info, warnings, and errors.
public static final Logger LOGGER = LogManager.getLogger("modid");

How does this sound? ```java // This logger is used to write text to the console and the log file. // It is considered best practice to use your mod id as the logger's name. // That way, it's clear which mod wrote info, warnings, and errors. public static final Logger LOGGER = LogManager.getLogger("modid"); ```