Skip to content

Conversation

@slawiko
Copy link
Contributor

@slawiko slawiko commented Mar 26, 2016

I think this is a typo

I think this is a typo
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 90.955% when pulling 0c56d80 on slawiko:patch-1 into 9074a9c on iluwatar:master.

@iluwatar
Copy link
Owner

In my opinion it is good to have the @Override annotation when implementing interface methods. This way we can utilize compiler checks to verify that the implementing method signature really matches the interface and is not a new method. We could enforce this with Checkstyle i.e. Checkstyle would give an error if interface method implementation would not specify @Override. @slawiko @npathai what do you think?

@slawiko
Copy link
Contributor Author

slawiko commented Mar 26, 2016

@iluwatar I thought it was already in your Checkstyle. Howbeit we MUST add it in checkstyle :)

@iluwatar
Copy link
Owner

@slawiko it is not there yet, otherwise the build would not work. Maybe it is this one: http://checkstyle.sourceforge.net/config_annotation.html#MissingOverride Could you try adding it and see what happens?

@slawiko
Copy link
Contributor Author

slawiko commented Mar 26, 2016

@iluwatar yes of course, but not right now.

@iluwatar
Copy link
Owner

No problem, just let us know then.

@slawiko
Copy link
Contributor Author

slawiko commented Mar 26, 2016

@iluwatar I read some information about Checkstyle and now I can do something. As I understood <module name="MissingOverride"/> works only if code contains JavaDoc tag {@inheritDoc}. But if I add this tag Imgur code inspection with Imgur still passes checkstyle: Imgur I use Checkstyle for the first time, so I can be wrong. By the way, I tried without JavaDoc tag {@inheritDoc}, but result was identical. What do I do wrong?

@iluwatar
Copy link
Owner

Looks like MissingOverride is not what we want. I'm not sure if Checkstyle has exactly what we need.

@iluwatar
Copy link
Owner

My suspicion is verified in http://checkstyle.sourceforge.net/google_style.html

"6.1 @OverRide: always used" - "That validation could not be checked by Checkstyle. It's need to take a look as parent class. But Checkstyle have no way open or look at another Class file. "

@iluwatar iluwatar merged commit d406f0f into iluwatar:master Mar 26, 2016
@iluwatar
Copy link
Owner

Thanks @slawiko for the improvement!

@slawiko
Copy link
Contributor Author

slawiko commented Mar 26, 2016

@iluwatar you're welcome!

@slawiko slawiko deleted the patch-1 branch March 27, 2016 06:29
@iluwatar iluwatar modified the milestone: 1.11.0 Apr 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants