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

Plugins should either have a builder or a factory method #3126

Open
ppkarwasz opened this issue Oct 27, 2024 · 2 comments
Open

Plugins should either have a builder or a factory method #3126

ppkarwasz opened this issue Oct 27, 2024 · 2 comments
Assignees
Labels
bug Incorrect, unexpected, or unintended behavior of existing code

Comments

@ppkarwasz
Copy link
Contributor

Log4j plugin should either have a builder annotated with @PluginBuilderFactory or a factory method annotated with @PluginFactory. As it turns out, some of them have both, for example PatternLayout:

@PluginFactory
@Deprecated
public static PatternLayout createLayout(

@PluginBuilderFactory
public static Builder newBuilder() {

This rule should probably be enforced by the annotation processor.

@ppkarwasz ppkarwasz added the bug Incorrect, unexpected, or unintended behavior of existing code label Oct 27, 2024
@ppkarwasz ppkarwasz self-assigned this Oct 27, 2024
ppkarwasz added a commit to apache/logging-log4j-transform that referenced this issue Oct 28, 2024
@vy
Copy link
Member

vy commented Oct 28, 2024

@ppkarwasz, agreed that one should be present, but since PBF was introduced after PF, it is inevitable that some classes have both, and I don't know of a way to fix this without breaking backward compatibility. Do you?

@ppkarwasz
Copy link
Contributor Author

@ppkarwasz, agreed that one should be present, but since PBF was introduced after PF, it is inevitable that some classes have both, and I don't know of a way to fix this without breaking backward compatibility. Do you?

I am not suggesting removing the factory methods, just the annotations (this is a MICRO change AFAIK). Having two annotations on the same class might lead to non deterministic behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect, unexpected, or unintended behavior of existing code
Projects
None yet
Development

No branches or pull requests

2 participants