You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/10/08 19:34:25 UTC
[GitHub] [commons-lang] arturobernalg opened a new pull request, #965: Remove unnecessary conditions. It's no need it after assigns tmp to duration
arturobernalg opened a new pull request, #965:
URL: https://github.com/apache/commons-lang/pull/965
We don't need that conditions. Its always false and will never reach it. And just because in the line 192 we assigns tmp to duration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] kinow commented on a diff in pull request #965: Remove unnecessary conditions. It's no need it after assigns tmp to duration
Posted by GitBox <gi...@apache.org>.
kinow commented on code in PR #965:
URL: https://github.com/apache/commons-lang/pull/965#discussion_r990698891
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -190,9 +190,6 @@ public static String formatDurationWords(
duration = tmp;
tmp = StringUtils.replaceOnce(duration, " 0 minutes", StringUtils.EMPTY);
duration = tmp;
- if (tmp.length() != duration.length()) {
- duration = StringUtils.replaceOnce(tmp, " 0 seconds", StringUtils.EMPTY);
- }
Review Comment:
Or is it a bug? Maybe a copypasta bug where accidentally lines 190 and 192 were duplicated? Because there are `replaceOne` for "hour" and "minute", and this block that was removed has the part for "seconds".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] arturobernalg commented on a diff in pull request #965: Remove unnecessary conditions. It's no need it after assigns tmp to duration
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on code in PR #965:
URL: https://github.com/apache/commons-lang/pull/965#discussion_r990793928
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -190,9 +190,6 @@ public static String formatDurationWords(
duration = tmp;
tmp = StringUtils.replaceOnce(duration, " 0 minutes", StringUtils.EMPTY);
duration = tmp;
- if (tmp.length() != duration.length()) {
- duration = StringUtils.replaceOnce(tmp, " 0 seconds", StringUtils.EMPTY);
- }
Review Comment:
Hi @kinow
Yes, could be a bug as well. In any case we don't need that conditions.
TY
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] codecov-commenter commented on pull request #965: Remove unnecessary conditions. It's no need it after assigns tmp to duration
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #965:
URL: https://github.com/apache/commons-lang/pull/965#issuecomment-1272402537
# [Codecov](https://codecov.io/gh/apache/commons-lang/pull/965?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#965](https://codecov.io/gh/apache/commons-lang/pull/965?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16cc08f) into [master](https://codecov.io/gh/apache/commons-lang/commit/b6d39a4257c46672e915098a156f63cd0a713a72?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6d39a4) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #965 +/- ##
============================================
+ Coverage 91.99% 92.01% +0.01%
- Complexity 7432 7433 +1
============================================
Files 189 189
Lines 15664 15662 -2
Branches 2907 2906 -1
============================================
+ Hits 14410 14411 +1
+ Misses 677 676 -1
+ Partials 577 575 -2
```
| [Impacted Files](https://codecov.io/gh/apache/commons-lang/pull/965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/commons/lang3/time/DurationFormatUtils.java](https://codecov.io/gh/apache/commons-lang/pull/965/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbGFuZzMvdGltZS9EdXJhdGlvbkZvcm1hdFV0aWxzLmphdmE=) | `91.10% <ø> (+0.76%)` | :arrow_up: |
| [...ommons/lang3/concurrent/AtomicSafeInitializer.java](https://codecov.io/gh/apache/commons-lang/pull/965/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbGFuZzMvY29uY3VycmVudC9BdG9taWNTYWZlSW5pdGlhbGl6ZXIuamF2YQ==) | `100.00% <0.00%> (+14.28%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory merged pull request #965: Remove unnecessary conditions. It's no need it after assigns tmp to duration
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #965:
URL: https://github.com/apache/commons-lang/pull/965
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org