Skip to content

Copy ome.api.* documentation to omero.api.* (1)#4547

Merged
joshmoore merged 9 commits intoome:developfrom
sbesson:ome_omero_apidoc
Apr 11, 2016
Merged

Copy ome.api.* documentation to omero.api.* (1)#4547
joshmoore merged 9 commits intoome:developfrom
sbesson:ome_omero_apidoc

Conversation

@sbesson
Copy link
Copy Markdown
Member

@sbesson sbesson commented Mar 29, 2016

See https://trello.com/c/KLCaZX4X/19-copy-ome-api-documentation-to-omero-api

This PR initializes the copy of the Javadoc documentation from the first set of classes of ome.api.* classes into the slice classes.

To review this PR, either build the slice documentation via slice2html or use the published CI documentation. Check the links are all working and there is no obvious error.

@jburel jburel added the develop label Mar 29, 2016
@ximenesuk
Copy link
Copy Markdown
Contributor

@sbesson sorry, I meant to look at this last week but failed to find the published CI docs and then forgot about it. Where are the published docs for the latest merge build?

@sbesson
Copy link
Copy Markdown
Member Author

sbesson commented Apr 4, 2016

@ximenesuk: see https://ci.openmicroscopy.org/view/DEV/job/OMERO-DEV-merge-build/ICE=3.5,jdk=8_LATEST,label=octopus/javadoc/slice2html/ or alternatively download the OMERO.docs artifacts which contains the API documentation.

@ximenesuk
Copy link
Copy Markdown
Contributor

@sbesson thanks, I was looking in all the wrong places - so long since I have had to look on Jenkins. I'll review tomorrow.

* @param group a new
* {@link omero.model.ExperimenterGroup} instance. Not null.
* @return id of the newly created {@link ExperimenterGroup}
* @see <a href="https://trac.openmicroscopy.org/ome/ticket/1434">ticket:1434"</a>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this is a copy across from ome but do you want to pick up minor fixes here? If so then there is a stray double quote:

ticket:1434" -> ticket:1434

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

More generally, none of the trac links resolve to anything. Is that expected in this build as they are external links? The source looks like the periods in the url are represented as double colons. Is that some intermediate representation or is it broken?

@ximenesuk
Copy link
Copy Markdown
Contributor

The copying on the whole looks fine. Some of my comments relate to minor errors that were in the original ome source. Other comments may well be because this isn't a fully rendered doc with css etc?

@sbesson
Copy link
Copy Markdown
Member Author

sbesson commented Apr 5, 2016

@ximenesuk: commits pushed to address your comments. A couple of comments:

  • it looks like @see does not cope well with external hyperlinks so I moved the Trac ticket references to the method descriptions instead
  • the @param leakage into the short description seemed to be fixed by adding final dots to the method description consistently with your observation in Copy ome.api.* documentation to omero.api.* (1) #4547 (comment).

* photo is present, the oldest version will be modified (i.e.
* the highest updateEvent id).
*
* Note: as outlined in ticket:1794, this photo will be placed
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This ticket reference needs to be a trac url

@ximenesuk
Copy link
Copy Markdown
Contributor

The four examples above are more of a question. The documentation doesn't match the code, should this be fixed as the move is made from ome -> omero or left for a later sweep?

@sbesson
Copy link
Copy Markdown
Member Author

sbesson commented Apr 6, 2016

@ximenesuk: I would be in favor or addressing only the mismatching @param PARAMETER here and leave the content/description in sync as we should review it together with ome.api.*. I can push a commit to fix the mismatching property names and the Trac ticket in this PR.

@ximenesuk
Copy link
Copy Markdown
Contributor

Looks good to merge.

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.

4 participants