Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Minor changes #206

Merged
merged 1 commit into from Feb 20, 2022
Merged

Minor changes #206

merged 1 commit into from Feb 20, 2022

Conversation

arturobernalg
Copy link
Member

@arturobernalg arturobernalg commented Jun 12, 2021 •
edited

  • Simplify conditions
  • Make final var

@coveralls
Copy link

coveralls commented Jun 12, 2021 •
edited

Coverage Status

Coverage increased (+0.02%) to 86.833% when pulling 7bc52c2 on arturobernalg:feature/minor_changes into 7c79a74 on apache:master.

@garydgregory
Copy link
Member

Please rebase on master. Recent changes should allow all builds to be green including Java 16 and 17-EA.

@garydgregory garydgregory changed the title Minor change: Minor changes Jun 12, 2021
@arturobernalg
Copy link
Member Author

Please rebase on master. Recent changes should allow all builds to be green including Java 16 and 17-EA.

Hi @garydgregory
done
TY

garydgregory
garydgregory reviewed Feb 14, 2022
View reviewed changes
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java Outdated
@@ -777,7 +777,7 @@ public String getName() {
@Override
public boolean isDirectory() {
final String n = getName();
return n != null && n.endsWith("/");
return n.endsWith("/");
Copy link
Member

Choose a reason for hiding this comment

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

It does seem that getName() can never be null, so maybe just return getName().endsWith("/"); ? Same for other getName() call sites?

Copy link
Member

Choose a reason for hiding this comment

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

ping?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed.
TY

@arturobernalg
Minor change:
852c760 
* Simplify conditions
* Make final var
@garydgregory garydgregory merged commit c936bba into apache:master Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garydgregory garydgregory Awaiting requested review from garydgregory

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@arturobernalg @coveralls @garydgregory