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

Fix a bug where builders of LogFormatter return internal classes #4975

Merged
merged 6 commits into from
Jun 21, 2023

Conversation

ikhoon
Copy link
Contributor

@ikhoon ikhoon commented Jun 21, 2023

Motivation:

TextLogFormatterBuilder.build() and JsonLogFormatterBuilder.build() return package-private classes that cause compilation errors when using them in different packages.

Modifications:

  • Make TextLogFormatterBuilder.build() and JsonLogFormatterBuilder.build() return LogFormatter instead of the internal classes.

Result:

You can no longer see compilation errors when using TextLogFormatterBuilder.build() and JsonLogFormatterBuilder.build().

Motivation:

`TextLogFormatterBuilder.build()` and `JsonLogFormatterBuilder.build()`
return package-private classes that cause compliation errors when using
them in different packages.

Modifications:

- Make `TextLogFormatterBuilder.build()` and `JsonLogFormatterBuilder.build()`
  return `LogFormatter` instead of the internal classes.

Result:

You can no longer see compilation errors when using
`TextLogFormatterBuilder.build()` and `JsonLogFormatterBuilder.build()`.
@ikhoon ikhoon added the defect label Jun 21, 2023
@ikhoon ikhoon added this to the 1.24.1 milestone Jun 21, 2023
Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix @ikhoon ! 👍 🙇 👍 (please check the CI failure)

Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

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

Thanks, @ikhoon ! 🙇

@ikhoon
Copy link
Contributor Author

ikhoon commented Jun 21, 2023

@minwoox PTAL. 🙇‍♂️

Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Thanks a lot for fixing this. 🙇

@ikhoon ikhoon merged commit e1cabd7 into line:main Jun 21, 2023
@ikhoon ikhoon deleted the logformater-public branch June 21, 2023 07:49
@ta7uw
Copy link
Member

ta7uw commented Jun 21, 2023

Thanks for quick fix
I'll be more careful🙇‍♂️

@minwoox
Copy link
Contributor

minwoox commented Jun 21, 2023

I'll be more careful

It was not you but me. 🤣

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

Successfully merging this pull request may close these issues.

5 participants