Skip to content
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

Add cp/_rm section for 5.0.7 #1079

Merged
merged 2 commits into from Jan 29, 2015
Merged

Add cp/_rm section for 5.0.7 #1079

merged 2 commits into from Jan 29, 2015

Conversation

joshmoore
Copy link
Member

@ximenesuk
Copy link
Contributor

This looks good but in looking at the built docs I noticed that the, not strictly in-place, transfer option "upload_rm" isn't yet documented so this might be a good PR for that to sit in. The comparison between the "cp" group and the "ln" group is fine but why "cp" over "upload"? That comparison should be stated somewhere hereabouts too.

@joshmoore
Copy link
Member Author

Happy to add in upload_*. Guess the only real optimization would be performance, though I haven't tested it. Assuming, say, a 1TB file, I'd think a simple cp would be much healthier.

@joshmoore
Copy link
Member Author

@ximenesuk : copied the minimal blurb from #975, but obviously you did much more there. Do we want to go further? /cc @hflynn

@ximenesuk
Copy link
Contributor

Looks good to me. A sentence hinting at the benefits of cp might head off any questions but if we have no concrete metrics then maybe leave that for now? I'm happy either way.

@joshmoore
Copy link
Member Author

I'm inclined to keep it a bit simple for 5.0.7. During the rebase, I'll see if it can be more tightly tied into your other fixes. @hflynn, this is all yours.

@hflynn
Copy link
Member

hflynn commented Jan 29, 2015

Erm, sorry to come at this late but if this isn't actually in-place import stuff, would it not be better documented on https://www.openmicroscopy.org/site/support/omero5-staging/sysadmins/import-scenarios.html or at least signposting further up the page ?

@joshmoore
Copy link
Member Author

cp and cp_rm is definitely in-place. upload and upload_rm work both via in-place and non-in-place (out-of-place? at-home?)

@hflynn
Copy link
Member

hflynn commented Jan 29, 2015

Ah ok, sorry, I obviously got the wrong end of the stick.

@hflynn
Copy link
Member

hflynn commented Jan 29, 2015

Re-running the merge build now but this is looking fine to me in that case. I'm inclined to agree about leaving the extra stuff on 5.1.0 only since we already decided not to rebase #975

@ximenesuk
Copy link
Contributor

Yes, I think in-place may now be a misplaced term! All 5.x imports are effectively in-place, it's just what you do before and after the import that differs!

@hflynn
Copy link
Member

hflynn commented Jan 29, 2015

We do like to pick our terminology!

@hflynn
Copy link
Member

hflynn commented Jan 29, 2015

Looks fine on staging. merging.

hflynn added a commit that referenced this pull request Jan 29, 2015
Add cp/_rm section for 5.0.7
@hflynn hflynn merged commit c01925a into ome:dev_5_0 Jan 29, 2015
@joshmoore joshmoore deleted the cp_rm branch January 29, 2015 15:47
@joshmoore
Copy link
Member Author

--rebased-to #1087

@sbesson sbesson added this to the 5.0.7 milestone Feb 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants