-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix #10175, #10953: shorten image name #1710
Conversation
Image.name: Good to merge. |
What is needed to make the javadoc targets happy? |
The build isn't setting the |
@@ -42,6 +42,7 @@ | |||
import omero.model.Image; | |||
import omero.model.Pixels; | |||
|
|||
import org.apache.commons.lang.StringUtils; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this import needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt it. (-: Will probably push a commit tomorrow to remove it.
@jburel: Interesting point. Happy to handle in this or a separate PR. |
Tested with Simon's supplied long image name - imports OK and the |
imageName = null; | ||
} | ||
if (userSpecifiedName != null) { | ||
saveName = userSpecifiedName.trim(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it only me, or is the trim()
method called twice on the userSpecifiedName
object, in this code path (cf. line 161)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thank you, the latter preceded the former. (-:
Not a blocker for merging this PR, but it would be nice if this code change came with a unit or integration test. |
Final commit looks good. Code makes sense now. Good to merge. |
fix #10175, #10953: shorten image name
Fixes http://trac.openmicroscopy.org.uk/ome/ticket/10175. To test, try importing images from absurdly long-named/deep paths. Cc: @manics.
--no-rebase as it's FS-specific.
Update: Additional test: Try importing images and check that the image name (also in DB as
image.name
) fixes https://trac.openmicroscopy.org.uk/ome/ticket/10953 butfilesetentry.clientpath
remains the full client-side path.