adoroszlai commented on code in PR #8124:
URL: https://github.com/apache/ozone/pull/8124#discussion_r2032892124
##########
hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/snapshot/ListSnapshotDiffHandler.java:
##########
@@ -37,38 +38,32 @@ public class ListSnapshotDiffHandler extends Handler {
@CommandLine.Mixin
private BucketUri snapshotPath;
- @CommandLine.Option(names = {"-s", "--status"},
+ @CommandLine.Option(names = {"-js", "--job-status"},
Review Comment:
Please avoid multi-char short options (`-js`).
- They should be combinable (`-j -s` should be the same as `-js`)
- Not every long option needs a short one
- Command-line completion is coming (#8030)
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java:
##########
@@ -731,19 +732,75 @@ public CancelSnapshotDiffResponse
cancelSnapshotDiff(String volumeName,
/**
* Get a list of the SnapshotDiff jobs for a bucket based on the JobStatus.
- * @param volumeName Name of the volume to which the snapshotted bucket
belong
- * @param bucketName Name of the bucket to which the snapshots belong
- * @param jobStatus JobStatus to be used to filter the snapshot diff jobs
- * @param listAll Option to specify whether to list all jobs or not
- * @return a list of SnapshotDiffJob objects
+ *
+ * @param volumeName Name of the volume to which the snapshotted
bucket belong
+ * @param bucketName Name of the bucket to which the snapshots
belong
+ * @param jobStatus JobStatus to be used to filter the snapshot
diff jobs
+ * @param listAll Option to specify whether to list all jobs or
not
+ * @param prevSnapshotDiffJob list snapshot diff jobs after this snapshot
diff job.
+ * @return an iterator of SnapshotDiffJob objects
* @throws IOException in case there is a failure while getting a response.
*/
- public List<OzoneSnapshotDiff> listSnapshotDiffJobs(String volumeName,
- String bucketName,
- String jobStatus,
- boolean listAll)
+ public Iterator<OzoneSnapshotDiff> listSnapshotDiffJobs(String volumeName,
+ String bucketName,
+ String jobStatus,
+ boolean listAll,
+ String
prevSnapshotDiffJob)
Review Comment:
This diff illustrates why we shouldn't indent method signature like this:
have to reindent all lines when only return type is changed.
Please avoid adding new methods formatted like this as well as changing
existing methods to be like this.
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java:
##########
@@ -731,19 +732,75 @@ public CancelSnapshotDiffResponse
cancelSnapshotDiff(String volumeName,
/**
* Get a list of the SnapshotDiff jobs for a bucket based on the JobStatus.
- * @param volumeName Name of the volume to which the snapshotted bucket
belong
- * @param bucketName Name of the bucket to which the snapshots belong
- * @param jobStatus JobStatus to be used to filter the snapshot diff jobs
- * @param listAll Option to specify whether to list all jobs or not
- * @return a list of SnapshotDiffJob objects
+ *
+ * @param volumeName Name of the volume to which the snapshotted
bucket belong
+ * @param bucketName Name of the bucket to which the snapshots
belong
+ * @param jobStatus JobStatus to be used to filter the snapshot
diff jobs
+ * @param listAll Option to specify whether to list all jobs or
not
+ * @param prevSnapshotDiffJob list snapshot diff jobs after this snapshot
diff job.
Review Comment:
No need to add whitespace.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
