Skip to content

Conversation

@sju
Copy link

@sju sju commented Jun 13, 2018

No description provided.

*
* @param job
*/
@Override
Copy link
Contributor

@sebastian-nagel sebastian-nagel Jun 13, 2018

Choose a reason for hiding this comment

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

+1 for adding missing @OverRide annotations.
This issue is the best example why @OverRide should be used: to avoid that by accident a method has not the intended signature.


public void cleanup() throws IOException {
@Override
public void cleanup(Context context) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also remove the method implementation. The superclass Reducer already implements already a do-nothing cleanup(context).

If you have time: there are a couple of other cleanup() methods without the context argument. Probably same mistake but harmless as they "do nothing". Need to check in detail but git grep -A2 'cleanup()' finds a couple of them. Thanks, @sju!

@sju
Copy link
Author

sju commented Jun 14, 2018

Did some cleanup ;-)

@sebastian-nagel sebastian-nagel merged commit 39da8c1 into apache:master Jun 21, 2018
@sebastian-nagel
Copy link
Contributor

Tested fix: the exception in the ResolverThreads is gone. Thanks, @sju!

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.

2 participants