You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@commons.apache.org by shahabkondri <gi...@git.apache.org> on 2018/01/14 19:53:29 UTC
[GitHub] commons-compress pull request #61: Code Cleanup.
GitHub user shahabkondri opened a pull request:
https://github.com/apache/commons-compress/pull/61
Code Cleanup.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shahabkondri/commons-compress cleanup
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-compress/pull/61.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #61
----
commit 182ccc1a418ec882270db5bc503aa5737265c9a9
Author: shahab <sh...@...>
Date: 2018-01-14T19:06:33Z
Modifier 'private' is redundant for enum constructors.
commit 4818cf0ccf606c2a4c0bca463d05c64b6bc404a8
Author: shahab <sh...@...>
Date: 2018-01-14T19:08:17Z
Modifier 'static' is redundant for enum constructors.
commit adb0b2ab641f629fe6da99a0a996eabff8b78911
Author: shahab <sh...@...>
Date: 2018-01-14T19:12:51Z
if statement can be simplified.
commit f5af518667398f02c78511247eb3338111fd8b92
Author: shahab <sh...@...>
Date: 2018-01-14T19:19:10Z
if statement can be simplified.
commit 30d624385def5739a5ec9eb1f6754b392c8392d2
Author: shahab <sh...@...>
Date: 2018-01-14T19:21:33Z
Local variable is redundant.
commit fef06ba8c650f2a2963dbbb23c2ba09e519da53d
Author: shahab <sh...@...>
Date: 2018-01-14T19:25:04Z
There is General exception 'IOException' in the throws list already.
commit f26634ce898eb1db4f71becf98b07bc3ebddb4d4
Author: shahab <sh...@...>
Date: 2018-01-14T19:26:56Z
remove unused import.
commit 068eefbae5a0df4000961e84a4dd37fe4bdb9f70
Author: shahab <sh...@...>
Date: 2018-01-14T19:32:18Z
Explicit manual boxing is unnecessary under Java 5 and newer, and can be safely removed.
commit 394a315316063d30769c89355732d57863d656cb
Author: shahab <sh...@...>
Date: 2018-01-14T19:37:59Z
'long' literal ending with lowercase 'l' instead of 'L'
----
---
[GitHub] commons-compress issue #61: Code Cleanup.
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/commons-compress/pull/61
[](https://coveralls.io/builds/15083738)
Coverage increased (+0.3%) to 86.384% when pulling **d8ffe597573680a00b16780ae83626e147232d76 on shahabkondri:cleanup** into **08cdbe2993fef2890e2b2609a653217e38fb67e0 on apache:master**.
---
[GitHub] commons-compress pull request #61: Code Cleanup.
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on a diff in the pull request:
https://github.com/apache/commons-compress/pull/61#discussion_r161847670
--- Diff: src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveEntry.java ---
@@ -179,12 +179,7 @@ public boolean equals(final Object obj) {
}
final ArArchiveEntry other = (ArArchiveEntry) obj;
if (name == null) {
- if (other.name != null) {
- return false;
- }
- } else if (!name.equals(other.name)) {
- return false;
- }
- return true;
+ return other.name == null;
+ } else return name.equals(other.name);
--- End diff --
please add braces for the else block.
---
[GitHub] commons-compress pull request #61: Code Cleanup.
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on a diff in the pull request:
https://github.com/apache/commons-compress/pull/61#discussion_r161847721
--- Diff: src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveEntry.java ---
@@ -886,12 +886,7 @@ public boolean equals(final Object obj) {
}
final CpioArchiveEntry other = (CpioArchiveEntry) obj;
if (name == null) {
- if (other.name != null) {
- return false;
- }
- } else if (!name.equals(other.name)) {
- return false;
- }
- return true;
+ return other.name == null;
+ } else return name.equals(other.name);
--- End diff --
please add braces for the else block.
---
[GitHub] commons-compress pull request #61: Code Cleanup.
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on a diff in the pull request:
https://github.com/apache/commons-compress/pull/61#discussion_r161848043
--- Diff: src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveEntry.java ---
@@ -418,12 +418,8 @@ public boolean equals(final Object o) {
}
// summary is always null right now, but this may change some day
- if ((summary == null && rhs.summary != null) // NOSONAR
- || (summary != null && !summary.equals(rhs.summary))) { // NOSONAR
- return false;
- }
-
- return true;
+ return (summary != null || rhs.summary == null) // NOSONAR
+ && (summary == null || summary.equals(rhs.summary));
}
--- End diff --
I find this version a lot harder to read than the original expression. I'm totally fine with getting rid of the `if`, but please keep the original logic.
---
[GitHub] commons-compress issue #61: Code Cleanup.
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on the issue:
https://github.com/apache/commons-compress/pull/61
merged, many thanks!
---
[GitHub] commons-compress issue #61: Code Cleanup.
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/commons-compress/pull/61
[](https://coveralls.io/builds/15045202)
Coverage increased (+0.1%) to 86.194% when pulling **394a315316063d30769c89355732d57863d656cb on shahabkondri:cleanup** into **08cdbe2993fef2890e2b2609a653217e38fb67e0 on apache:master**.
---
[GitHub] commons-compress pull request #61: Code Cleanup.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/commons-compress/pull/61
---
[GitHub] commons-compress pull request #61: Code Cleanup.
Posted by shahabkondri <gi...@git.apache.org>.
Github user shahabkondri commented on a diff in the pull request:
https://github.com/apache/commons-compress/pull/61#discussion_r161861720
--- Diff: src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveEntry.java ---
@@ -418,12 +418,8 @@ public boolean equals(final Object o) {
}
// summary is always null right now, but this may change some day
- if ((summary == null && rhs.summary != null) // NOSONAR
- || (summary != null && !summary.equals(rhs.summary))) { // NOSONAR
- return false;
- }
-
- return true;
+ return (summary != null || rhs.summary == null) // NOSONAR
+ && (summary == null || summary.equals(rhs.summary));
}
--- End diff --
I've double checked and reverted to the original logic.
---
[GitHub] commons-compress issue #61: Code Cleanup.
Posted by shahabkondri <gi...@git.apache.org>.
Github user shahabkondri commented on the issue:
https://github.com/apache/commons-compress/pull/61
Thanks for reviewing.
The suffix L is preferred, because the letter l (ell) is often hard to distinguish from the digit 1 (one).
---