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

Cleanup debug log handling #21

Merged
merged 2 commits into from Jun 3, 2019
Merged

Cleanup debug log handling #21

merged 2 commits into from Jun 3, 2019

Conversation

rhowe
Copy link
Contributor

@rhowe rhowe commented May 26, 2019

Make better use of isDebugEnabled(). See individual commits for details.

The diff looks better if you ignore whitespace changes.

@@ -960,7 +963,7 @@ else if ( CompilerConfiguration.CompilerReuseStrategy.ReuseSame.getStrategy().eq
}

String[] cl = compiler.createCommandLine( compilerConfiguration );
if ( getLog().isDebugEnabled() && cl != null && cl.length > 0 )
if ( cl != null && cl.length > 0 )
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to keep the getLog().isDebugEnabled() here, otherwise a lot a String concatenation is done for showing the commandline, which is only shown during debug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good catch!

@rhowe
Copy link
Contributor Author

rhowe commented Jun 3, 2019

Should this change have a Jira ticket?

@rfscholte rfscholte merged commit 24141bd into apache:master Jun 3, 2019
@rfscholte
Copy link
Contributor

I've merged it, I don't think we'll make anybody happy when such tiny improvements end up in the release notes.

@rhowe rhowe deleted the debug-cleanup branch June 4, 2019 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants