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

Restrict visibility of methods in abstract classes of jacoco-maven-plugin #175

Merged
merged 2 commits into from Dec 20, 2013

Conversation

Projects
None yet
2 participants
@mfriedenhagen
Member

mfriedenhagen commented Dec 20, 2013

Hi Mirko,

with the new abstract base classes we got some compiler warnings in the Eclipse IDE due to missing JavaDoc. In general I prefer to have all projects warning free in the Eclipse workspace.

Our IDE settings require JavaDoc for all API elements. In the jacoco-maven-plugin we got new protected methods without JavaDoc. I see to options to fix this:

  1. Add missing JavaDoc to protected methods
  2. Should these methods really be API? If not we can just make them package private.

Cheers,
-marc

@ghost ghost assigned mfriedenhagen Dec 20, 2013

@mfriedenhagen

This comment has been minimized.

Show comment
Hide comment
@mfriedenhagen

mfriedenhagen Dec 20, 2013

Member

@marcof: I agree completely. I normally use checkstyle to help with this. I
will take a look. I do not like protected very much and normally use
package scope. This allows testability and some code reuse in the same jar
without widening the API for others to much. For other stuff I prefer
interfaces and composition :-)

Member

mfriedenhagen commented Dec 20, 2013

@marcof: I agree completely. I normally use checkstyle to help with this. I
will take a look. I do not like protected very much and normally use
package scope. This allows testability and some code reuse in the same jar
without widening the API for others to much. For other stuff I prefer
interfaces and composition :-)

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Dec 20, 2013

Member

@mfriedenhagen BTW, do the shared Eclipse workspace settings now work for you? In this case you should see the warnings.

Member

marchof commented Dec 20, 2013

@mfriedenhagen BTW, do the shared Eclipse workspace settings now work for you? In this case you should see the warnings.

mfriedenhagen added a commit that referenced this pull request Dec 20, 2013

Merge pull request #175 from jacoco/issue-175
Eclipse Compiler Warnings in jacoco-maven-plugin

@mfriedenhagen mfriedenhagen merged commit bfcc0db into master Dec 20, 2013

1 check passed

default The Travis CI build passed
Details
@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Dec 21, 2013

Member

@mfriedenhagen Thanks! All warnings gone now.

May I ask you for a small adjustment: We decided not to use @author tags. Can we replace the author tags with a short description of the classes AbstractAgentMojo and AbstractReportMojo?

Member

marchof commented Dec 21, 2013

@mfriedenhagen Thanks! All warnings gone now.

May I ask you for a small adjustment: We decided not to use @author tags. Can we replace the author tags with a short description of the classes AbstractAgentMojo and AbstractReportMojo?

@mfriedenhagen

This comment has been minimized.

Show comment
Hide comment
@mfriedenhagen

mfriedenhagen Dec 21, 2013

Member

Will do on master

Member

mfriedenhagen commented Dec 21, 2013

Will do on master

@mfriedenhagen mfriedenhagen deleted the issue-175 branch Dec 21, 2013

mfriedenhagen added a commit that referenced this pull request Dec 21, 2013

@jacoco jacoco locked and limited conversation to collaborators Jan 11, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.