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

HHH-13133 #2941

Closed
wants to merge 2 commits into from
Closed

HHH-13133 #2941

wants to merge 2 commits into from

Conversation

skis
Copy link

@skis skis commented Jul 14, 2019

Pull request to HHH-13133

@@ -157,7 +157,7 @@ public boolean doExtendedEnhancement(UnloadedClass classDescriptor) {

writeOutEnhancedClass( enhancedBytecode, file );

getLog().info( "Successfully enhanced class [" + file + "]" );
Copy link
Member

Choose a reason for hiding this comment

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

could you change this to make sure the strings aren't appended when the logger is not set to debug?

Something like

log = getLog()

if (log.isDebugEnabled()) { log.debug( .... ) }

Copy link
Author

Choose a reason for hiding this comment

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

I will make the change and submit another PR.

@Sanne
Copy link
Member

Sanne commented Aug 2, 2019

Many thanks! Only had a very minor comment, if you could address that I'll merge it right away.

Generally speaking, try to not include unrelated changes in the same commit / same JIRA id.. but it's fine since they are very minor.

@Sanne
Copy link
Member

Sanne commented Aug 5, 2019

merged it. Thanks!

@Sanne Sanne closed this Aug 5, 2019
@skis
Copy link
Author

skis commented Aug 6, 2019

Thanks for merging Sanne. Strangely I didn't get any notification for the last 4 days.

@Sanne
Copy link
Member

Sanne commented Aug 6, 2019

@skis no worries, and no need to make another PR as I already changed it!

Thanks again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants