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

Expose upload_rm option in the documentation #975

Merged
merged 7 commits into from
Sep 10, 2014

Conversation

ximenesuk
Copy link
Contributor

This is a doc change explaining the upload_rm cli import option. It was originally written as a 5.0.4 fix and so made minimal changes to pages. It may be that for 5.1 we will want to further re-organise the import pages.

@sbesson This needs the file /downloads/inplace/advanced-help.txt pulling in from somewhere. Is there an automatic way to do this?

@sbesson
Copy link
Member

sbesson commented Sep 1, 2014

@ximenesuk: you can either cherry-pick the commit auto-generated by OMERO-5-1-merge-docs-autogen after ome/openmicroscopy#2999 is included into the merge build or wait until this upstream PR is merged into develop and use the commit auto-generated by OMERO-5.1-latest-docs-autogen.

@ximenesuk
Copy link
Contributor Author

@sbesson Thanks! I'll wait on this one being merged. Hopefully that won't be a problem but I won't tempt fate.

.. _upload_dropbox_auto:

DropBox import (automatic delete)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awfully long underline

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think the heading may have been shortened!

@@ -222,7 +245,7 @@ In-place DropBox import (automatic delete)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

One option that only exists in the in-place scenario is to have
files removed from DropBox automatically after a successful delete.
files removed from DropBox automatically after a successful import.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still actually true if upload_rm isn't in-place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the use of "only"? Probably not completely true, now.

@sbesson
Copy link
Member

sbesson commented Sep 2, 2014

@ximenesuk
Copy link
Contributor Author

Thanks @sbesson - there's no problem with that commit including other changes?

@sbesson
Copy link
Member

sbesson commented Sep 2, 2014

@ximenesuk: no, it should be fine. This commit should only include changes which have been merged in the upstream repository.

ximenesuk and others added 2 commits September 3, 2014 13:41
Repository: openmicroscopy/ome-documentation
Already up-to-date.

Generated by OMERO-5.1-latest-docs-autogen#86 (http://ci.openmicroscopy.org/job/OMERO-5.1-latest-docs-autogen/86/)
@sbesson
Copy link
Member

sbesson commented Sep 4, 2014

@ximenesuk
Copy link
Contributor Author

@mtbc Any further comments on the changes?

@mtbc
Copy link
Member

mtbc commented Sep 5, 2014

I'd still add a couple of "much faster" words for https://github.com/openmicroscopy/ome-documentation/pull/975/files#r16982141 but apart from that the latest changes look good.

@hflynn hflynn added the develop label Sep 8, 2014
@jburel
Copy link
Member

jburel commented Sep 10, 2014

Please read and understand the implications of the ln_rm option.
I will replace by
Please read carefully the implications of using the ln_rm option.

@ximenesuk
Copy link
Contributor Author

@jburel Although this line is from the orignal page (ie I didn't think to address it in this PR) it could do with tweaking. However, I'm not sure that they are reading the implications but making them based on reading the docs. So maybe something more like:

Please read carefully <link to "ln_rm" section just above> to fully understand the implications of using the this option.

/cc @joshmoore

@jburel
Copy link
Member

jburel commented Sep 10, 2014

@ximenesuk: your suggestion really highlights the fact that people need to be careful. (excluding the "the this")

@hflynn
Copy link
Contributor

hflynn commented Sep 10, 2014

I'd favour Please read <link> carefully to ensure you fully understand the implications of using this option.

@ximenesuk
Copy link
Contributor Author

I'm happy with Helen's wording. My "the this" was a typo - honest :-)

@ximenesuk
Copy link
Contributor Author

Better? The link looks a bit odd as we have used quotes and lower case in the various subheadings. The subheadings might be better rewritten but at the same time I don't really want this PR to last for ever :-)

@hflynn
Copy link
Contributor

hflynn commented Sep 10, 2014

Looks fine to me, are we ready to merge now?

@jburel
Copy link
Member

jburel commented Sep 10, 2014

I think so.

hflynn pushed a commit that referenced this pull request Sep 10, 2014
Expose upload_rm option in the documentation
@hflynn hflynn merged commit 01a91b2 into ome:develop Sep 10, 2014
@hflynn
Copy link
Contributor

hflynn commented Sep 10, 2014

Is there a plan to rebase the code for this if we do a 5.0.5 for OMERO?

@joshmoore
Copy link
Member

I'd leave that at your discretion, @hflynn, though will have to be fairly careful with the likes of --summary (i.e. 5.1 features slipping in)

@hflynn
Copy link
Contributor

hflynn commented Sep 10, 2014

Ok, probably safer to leave it then.

@hflynn
Copy link
Contributor

hflynn commented Sep 10, 2014

--no-rebase

@sbesson sbesson added this to the 5.1.0-m1 milestone Oct 14, 2014
joshmoore added a commit to joshmoore/ome-documentation that referenced this pull request Jan 29, 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

7 participants