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 eclipse warnings #1916

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

fix eclipse warnings #1916

wants to merge 6 commits into from

Conversation

rzpt
Copy link
Contributor

@rzpt rzpt commented Nov 16, 2022

Only generated _plainSerDe when it is used.

Partial fix for #1635

Before this PR

Generated code has a lot of warnings in eclipse.

After this PR

==COMMIT_MSG==
Fix unused warnings in eclipse
==COMMIT_MSG==

Possible downsides?

Untested in intellij since I don't use it. Only tested on a single internal repo.

Remove @SuppressWarnings("deprecation") on deprecated methods. The
annotation is for suppressing calls of deprecated methods, not
implementation of deprecated methods.

Only generated _plainSerDe when it is used.

Fixes #1635
@changelog-app
Copy link

changelog-app bot commented Nov 16, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Fix warnings in eclipse

Check the box to generate changelog(s)

  • Generate changelog entry

@@ -75,6 +74,7 @@ public final class DefaultStaticFactoryMethodGenerator implements StaticFactoryM
private final ParameterTypeMapper parameterTypes;
private final ReturnTypeMapper returnTypes;
private final StaticFactoryMethodType methodType;
private final ThreadLocal<Boolean> shouldGeneratePlainSerDe = ThreadLocal.withInitial(() -> false);
Copy link
Contributor

Choose a reason for hiding this comment

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

ThreadLocals have a bunch of gotchas that can be really troublesome to debug. Is there a reason we can't pass info up and down the stack to set this as needed all within the generate() method and its calls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

methodBuilder.addAnnotation(AnnotationSpec.builder(SuppressWarnings.class)
.addMember("value", "$S", "deprecation")
.build());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this change to remove the SuppressWarnings annotations seems independent from the plainSerDe change. Can they be separate PRs, or are they interrelated somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved deprecation stuff to #1917

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

Successfully merging this pull request may close these issues.

3 participants