-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 warnings #2014
Fix warnings #2014
Conversation
@@ -35,7 +35,6 @@ | |||
<groupId>org.apache.maven.plugins</groupId> | |||
<artifactId>maven-javadoc-plugin</artifactId> | |||
<configuration> | |||
<includePackageNames>com.google.gson</includePackageNames> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was detected by Eclipse; the maven-javadoc-plugin
does not have a parameter with this name (anymore?).
0823478
to
13a58bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This is good for hygiene.
@@ -26,8 +26,8 @@ | |||
* @author Joel Leitch | |||
*/ | |||
public class InnerClassExclusionStrategyTest extends TestCase { | |||
public InnerClass innerClass = new InnerClass(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this change is great. Previously the test showed that this field was excluded, despite being public. Now it's still excluded, but maybe that's because the field is private. It might be better to add comments saying why the fields are public.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I have reverted the changes for this test class.
* Fix warnings * Address review feedback
Note: The visibility changes in the test classes are not strictly necessary. Eclipse IDE complained that the exposed types had lower visibility than the exposing field.