[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4936: Core, API: Add support for renaming tags

[GitBox <gi...@apache.org>]

amogh-jahagirdar commented on code in PR #4936:
URL: https://github.com/apache/iceberg/pull/4936#discussion_r887383879


##########
core/src/main/java/org/apache/iceberg/UpdateSnapshotReferencesOperation.java:
##########
@@ -85,16 +86,29 @@ public UpdateSnapshotReferencesOperation removeTag(String name) {
   }
 
   public UpdateSnapshotReferencesOperation renameBranch(String name, String newName) {
-    Preconditions.checkNotNull(name, "Branch to rename cannot be null");
-    Preconditions.checkNotNull(newName, "New branch name cannot be null");
+    renameRef(name, newName, SnapshotRefType.BRANCH);
+    return this;
+  }
+
+  public UpdateSnapshotReferencesOperation renameTag(String name, String newName) {
+    renameRef(name, newName, SnapshotRefType.TAG);
+    return this;
+  }
+
+  private void renameRef(String name, String newName, SnapshotRefType type) {

Review Comment:
   Still don't really know about this helper method. I added it to reduce code duplication for renameBranch/renameTag as they are basically the same (check if the ref exists and it's the expected type. Then verify the target ref doesn't already exist. Remove the old ref and add the new ref), it's just the messaging that's different and we expose the branch/tag specific methods so it's more explicit to the user of the API .
   
   Code duplication is reduced but there's branching logic just for the different names in the message which seems overkill. May be worth the code duplication if the community feels it's more readable that way



-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org