Fix Java warnings excluding generated-sources

GiraffaFS / giraffa

Giraffa FileSystem (Slack: giraffa-fs.slack.com)

https://giraffa.ci.cloudbees.com

Apache License 2.0

17 stars 6 forks source link

Closed shvachko closed 9 years ago

shvachko commented 9 years ago

We have a lot of Java warnings. Fix them excluding WebUI for now.

octo47 commented 9 years ago

Fixed what Idea suggests and seems reasonable. Didn't touch BlockManagementAgent and NamespaceAgent due of upcoming #94

shvachko commented 9 years ago

There is a lot of unrelated changes, like

changing qualifiers of members and variables. They may or may not be correct. Say, UTF8 is a constant, therefore must be final static, which you removed. But let's stick to the changes, which fix warnings ONLY here.
reformatting imports, as in GiraffaHbaseServlet. We try not do that as different IDEs format them differently. In this case thought GiraffaHbaseServlet belongs to the web package and thus is beyond the scope of this issue.

So the patch needs some cleaning up.

octo47 commented 9 years ago

sure, qualifiers are more style fixes then warnings. interfaces usually static fields. but let's be explicit. as for ordering we need to define style to avoid ordering by hand. something like https://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s3.3-import-statements I'll revert that back.

octo47 commented 9 years ago

fixed, thank you for reviewing.

shvachko commented 9 years ago

Patch adds a bogus comment in GiraffaFileSystem. Please remove. //noinspection StatementWithEmptyBody
Also there is no need to @SuppressWarnings("deprecation"). I don't see any warnings in GiraffaFileSystem.
@SuppressWarnings("ALL") is cheating - doesn't work. If there is no way to fix, just leave them with a proper explanation. But I don't see any warnings GiraffaProtos.
There are no warnings in RowKeyBytes. Don't change it. You can fix it in another jira if this is a bug, which I personally don't see.
Changes in GiraffaFileServlet, GiraffaHbaseServlet and GiraffaWebUtils are in web package, should not be in this jira.
TestDirectoryTable does not have any warnings, why did you change it?
TestGiraffaUpgrade still has two warnings: unused blockID and genStamp.

octo47 commented 9 years ago

I used Intellij warnings, so that is sort of combined issue with #97. I'd rather will not do this issue. Sorry to taking it.

shvachko commented 9 years ago

I updated issue-104 branch, but did not create a new pull request. Please review the branch, just compare it to trunk.

I fixed (Eclipse) warnings including the web package, but excluding generated-sources.
Moved access to getClient() into a new HDFSAdapter class. We should eventually remove it, as direct access to these things will not be needed whith the new BlockManager.
Fixed GiraffaFileServlet, GiraffaHbaseServlet and GiraffaWebUtils warnings as well. Will update the issue title respectively.

pjeli commented 9 years ago

+1; tests passing and changes look good.

shvachko commented 9 years ago

Committed. Thanks everybody for working on it.