-
Notifications
You must be signed in to change notification settings - Fork 38
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
Code Improvements #36
Conversation
3385226
to
5645cee
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.
Some changes look good as they resolve problems.
- Using latest URIs (Old URIs will outdate)
- Removing unused imports (they should cause warnings).
But other changes look unnecessary as original codes don’t cause problems and don’t considered wrong. I prefer preserving original codes as far as it doesn’t cause problems.
I don’t know much about latest JDKs and let me know if those original codes cause errors or warnings.
Most of the changes (102059e, e0ecb9d, bdc6d9b, 17c3074, 1009d30, e4206b5) are based on findings from IntelliJ IDEA. The rest (311de6b, 5645cee) are additional suggestions from myself. For me all of them improve the code in some way. Reducing warnings, using more up-to-date Java syntax/features, more consistency, ... Can you name the commits you are willing to merge? Then I'll remove the other ones. |
I don’t use IntelliJ and I’m not sure where those warnings come from (javac, findbugs, or other lint tools). Following commits are acceptable to me without any considerations: |
5645cee
to
749588a
Compare
The pull request now only contains the two commits we agreed on. |
This pull request contains multiple commits, each covering a different aspect and dedicated commit message. It's therefore also a good idea to have a look at each commit individually.