You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/06/10 16:02:23 UTC
[GitHub] [hadoop-ozone] bhemanthkumar opened a new pull request #1051: Redundancy if condition code in ListPipelinesSubcommand
bhemanthkumar opened a new pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051
Remove the redundancy if condition code in ListPipelinesSubcommand
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
## What is the link to the Apache JIRA
(Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)
Please replace this section with the link to the Apache JIRA)
## How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on a change in pull request #1051: Redundancy if condition code in ListPipelinesSubcommand
Posted by GitBox <gi...@apache.org>.
maobaolong commented on a change in pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051#discussion_r442617207
##########
File path: hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/pipeline/ListPipelinesSubcommand.java
##########
@@ -54,17 +54,12 @@
@Override
public Void call() throws Exception {
try (ScmClient scmClient = parent.getParent().createScmClient()) {
- if (Strings.isNullOrEmpty(factor) && Strings.isNullOrEmpty(state)) {
- scmClient.listPipelines().forEach(System.out::println);
- } else {
- scmClient.listPipelines().stream()
- .filter(p -> ((Strings.isNullOrEmpty(factor) ||
- (p.getFactor().toString().compareToIgnoreCase(factor) == 0))
- && (Strings.isNullOrEmpty(state) ||
- (p.getPipelineState().toString().compareToIgnoreCase(state)
- == 0))))
- .forEach(System.out::println);
- }
+ scmClient.listPipelines().stream()
+ .filter(p -> ((Strings.isNullOrEmpty(factor)
+ || (p.getFactor().toString().compareToIgnoreCase(factor) == 0))
+ && (Strings.isNullOrEmpty(state) || (p.getPipelineState()
+ .toString().compareToIgnoreCase(state) == 0))))
+ .forEach(System.out::println);
Review comment:
@bhemanthkumar i think the changes within the try block @adoroszlai is
```java
Stream<Pipeline> stream = scmClient.listPipelines().stream();
if (!Strings.isNullOrEmpty(factor)) {
stream =
stream.filter(p -> p.getFactor().toString()
.compareToIgnoreCase(factor) == 0);
}
if (!Strings.isNullOrEmpty(state)) {
stream =
stream.filter(p -> p.getPipelineState().toString()
.compareToIgnoreCase(state) == 0);
}
stream.forEach(System.out::println);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1051: Redundancy if condition code in ListPipelinesSubcommand
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051#discussion_r439741987
##########
File path: hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/pipeline/ListPipelinesSubcommand.java
##########
@@ -54,17 +54,12 @@
@Override
public Void call() throws Exception {
try (ScmClient scmClient = parent.getParent().createScmClient()) {
- if (Strings.isNullOrEmpty(factor) && Strings.isNullOrEmpty(state)) {
- scmClient.listPipelines().forEach(System.out::println);
- } else {
- scmClient.listPipelines().stream()
- .filter(p -> ((Strings.isNullOrEmpty(factor) ||
- (p.getFactor().toString().compareToIgnoreCase(factor) == 0))
- && (Strings.isNullOrEmpty(state) ||
- (p.getPipelineState().toString().compareToIgnoreCase(state)
- == 0))))
- .forEach(System.out::println);
- }
+ scmClient.listPipelines().stream()
+ .filter(p -> ((Strings.isNullOrEmpty(factor)
+ || (p.getFactor().toString().compareToIgnoreCase(factor) == 0))
+ && (Strings.isNullOrEmpty(state) || (p.getPipelineState()
+ .toString().compareToIgnoreCase(state) == 0))))
+ .forEach(System.out::println);
Review comment:
Structuring this as below would reduce duplication, and make it easier to add further filters if needed:
```
Stream<...> stream = scmClient.listPipelines().stream();
if (!isNullOrEmpty(factor)) {
stream = stream.filter(...);
}
if (!isNullOrEmpty(state)) {
stream = stream.filter(...);
}
stream.forEach(...);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on pull request #1051: Redundancy if condition code in ListPipelinesSubcommand
Posted by GitBox <gi...@apache.org>.
maobaolong commented on pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051#issuecomment-642148224
@bhemanthkumar Thanks for working on this, please fix the style problem. Also, please update the description from the given template.
Reference this PR. https://github.com/apache/hadoop-ozone/pull/920
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hemanthboyina commented on pull request #1051: HDDS-3747. Redundancy if condition code in ListPipelinesSubcommand
Posted by GitBox <gi...@apache.org>.
hemanthboyina commented on pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051#issuecomment-654332726
@dineshchitlangia can we push this forward
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] codecov-commenter commented on pull request #1051: Redundancy if condition code in ListPipelinesSubcommand
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051#issuecomment-643451424
# [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1051?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@1238769`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hadoop-ozone/pull/1051?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1051 +/- ##
=========================================
Coverage ? 69.37%
Complexity ? 9107
=========================================
Files ? 961
Lines ? 48125
Branches ? 4675
=========================================
Hits ? 33388
Misses ? 12523
Partials ? 2214
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1051?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1051?src=pr&el=footer). Last update [1238769...45d9b27](https://codecov.io/gh/apache/hadoop-ozone/pull/1051?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on pull request #1051: HDDS-3747. Redundancy if condition code in ListPipelinesSubcommand
Posted by GitBox <gi...@apache.org>.
maobaolong commented on pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051#issuecomment-652765222
@dineshchitlangia Would you like to merge this PR for @bhemanthkumar ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on a change in pull request #1051: Redundancy if condition code in ListPipelinesSubcommand
Posted by GitBox <gi...@apache.org>.
maobaolong commented on a change in pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051#discussion_r438285600
##########
File path: hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/pipeline/ListPipelinesSubcommand.java
##########
@@ -54,17 +54,13 @@
@Override
public Void call() throws Exception {
try (ScmClient scmClient = parent.getParent().createScmClient()) {
- if (Strings.isNullOrEmpty(factor) && Strings.isNullOrEmpty(state)) {
- scmClient.listPipelines().forEach(System.out::println);
- } else {
- scmClient.listPipelines().stream()
- .filter(p -> ((Strings.isNullOrEmpty(factor) ||
- (p.getFactor().toString().compareToIgnoreCase(factor) == 0))
- && (Strings.isNullOrEmpty(state) ||
- (p.getPipelineState().toString().compareToIgnoreCase(state)
- == 0))))
+ scmClient.listPipelines().stream()
Review comment:
Please reduce the indent to fix the checkstyle failure.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai merged pull request #1051: HDDS-3747. Redundancy if condition code in ListPipelinesSubcommand
Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on pull request #1051: Redundancy if condition code in ListPipelinesSubcommand
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #1051:
URL: https://github.com/apache/hadoop-ozone/pull/1051#issuecomment-645174652
@bhemanthkumar , can you please fill up the PR details as requested in the template?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org