Skip to content

Refactor: Replace System.out/err with pcgen.util.Logging to resolve PMD warnings - #7686

Open
cpstence wants to merge 1 commit into
PCGen:masterfrom
cpstence:master
Open

Refactor: Replace System.out/err with pcgen.util.Logging to resolve PMD warnings#7686
cpstence wants to merge 1 commit into
PCGen:masterfrom
cpstence:master

Conversation

@cpstence

@cpstence cpstence commented Aug 4, 2026

Copy link
Copy Markdown

…MD warnings

@Vest

@Vest
Vest self-requested a review August 4, 2026 21:17
@Vest Vest self-assigned this Aug 4, 2026
@Vest

Vest commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

few comments:
if you can use "import", do it.
Logging.errorPrint is better than pcgen.util.Logging.errorPrint (everywhere).
The same goes to java.util.logging.Level.INFO, we have some constants in Logging as well.

These were my findings, and here are from my bot:

ShowMessageDelegate is a behavior risk. That println is the headless/batch fallback for showing a message when there are no GUI observers. Routing it to Logging.log(INFO) means it can be filtered/suppressed by log config — a user-facing message in non-GUI mode could silently disappear. Needs care, not a blind swap.

Tips.java half-converted & inconsistent. It's a standalone dev/translation tool that legitimately uses System.out — it still keeps e.printStackTrace() and the now-inaccurate @SuppressWarnings("UseOfSystemOutOrSystemErr"). Either leave it fully alone or clean it properly; don't do half.

I need you to leave suppresswarnings and don't touch this and/or you can analyze the Tips.java file, check where it is used, how and see, whether my bot's comment is applicable here.

Regarding the ShowMessageDelegate -- this is serious, I agree with the bot: try to run integration tests (you can try this with the debugger, or change the code like "Vest was here and said: " + whatever is in the code and read the logs, whether your fix doesn't break anything.

@Vest Vest changed the title Refactor: Replace System.out/err with pcgen.util.Logging to resolve P… Refactor: Replace System.out/err with pcgen.util.Logging to resolve PMD warnings Aug 4, 2026

@Vest Vest left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text is in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants