You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "matthias-koch (via GitHub)" <gi...@apache.org> on 2023/09/14 14:02:28 UTC

[GitHub] [zeppelin] matthias-koch opened a new pull request, #4658: [ZEPPELIN-5964] Fix renaming bug

matthias-koch opened a new pull request, #4658:
URL: https://github.com/apache/zeppelin/pull/4658

   ### What is this PR for?
   If a note is renamed by changing only the capitalization, the note is copied at the file system level when it is executed again.
   
   ### What type of PR is it?
   Bug Fix
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/browse/ZEPPELIN-5964
   
   ### How should this be tested?
   * New unit test were added
   
   ### Questions:
   * Does the license files need to update?
   * Is there breaking changes for older versions?
   * Does this needs documentation?
   


-- 
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: reviews-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] Reamer merged pull request #4658: [ZEPPELIN-5964] Fix renaming bug

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer merged PR #4658:
URL: https://github.com/apache/zeppelin/pull/4658


-- 
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: reviews-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] matthias-koch commented on a diff in pull request #4658: [ZEPPELIN-5964] Fix renaming bug

Posted by "matthias-koch (via GitHub)" <gi...@apache.org>.
matthias-koch commented on code in PR #4658:
URL: https://github.com/apache/zeppelin/pull/4658#discussion_r1329929386


##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java:
##########
@@ -247,7 +247,7 @@ public void moveNote(String noteId,
     this.notebookRepo.move(noteId, notePath, newNotePath, subject);
 
     // Update path of the note
-    if (!StringUtils.equalsIgnoreCase(notePath, newNotePath)) {
+    if (!notePath.equals(newNotePath)) {

Review Comment:
   Git: case sensitive
   Hadoop: case sensitive
   S3: case sensitive
   Azure: case sensitive
   Google Cloud Storage: case sensitive
   Aliyun OSS: case sensitive
   MongoDB: case sensitive
   Github: case sensitive
   
   It seems we are safe.



-- 
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: reviews-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] Reamer commented on a diff in pull request #4658: [ZEPPELIN-5964] Fix renaming bug

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4658:
URL: https://github.com/apache/zeppelin/pull/4658#discussion_r1326124188


##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java:
##########
@@ -247,7 +247,7 @@ public void moveNote(String noteId,
     this.notebookRepo.move(noteId, notePath, newNotePath, subject);
 
     // Update path of the note
-    if (!StringUtils.equalsIgnoreCase(notePath, newNotePath)) {
+    if (!notePath.equals(newNotePath)) {

Review Comment:
   `StringUtils.equals(x, y)` is null safe and should be preferred.



-- 
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: reviews-unsubscribe@zeppelin.apache.org

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


[GitHub] [zeppelin] Reamer commented on a diff in pull request #4658: [ZEPPELIN-5964] Fix renaming bug

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4658:
URL: https://github.com/apache/zeppelin/pull/4658#discussion_r1330037065


##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java:
##########
@@ -247,7 +247,7 @@ public void moveNote(String noteId,
     this.notebookRepo.move(noteId, notePath, newNotePath, subject);
 
     // Update path of the note
-    if (!StringUtils.equalsIgnoreCase(notePath, newNotePath)) {
+    if (!notePath.equals(newNotePath)) {

Review Comment:
   awesome



-- 
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: reviews-unsubscribe@zeppelin.apache.org

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