Skip to content

Conversation

@snwoods
Copy link
Contributor

@snwoods snwoods commented Jun 17, 2025

No description provided.

@psafont
Copy link
Member

psafont commented Jun 17, 2025

We probably want to add some documentation-comments to the type signatures here

Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

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

Some functions have multiple string parameters, no labels, and no documentation.

@snwoods snwoods force-pushed the private/stevenwo/observer_helpers_mli branch from 8ec5b90 to 663c08d Compare June 25, 2025 09:03
@snwoods
Copy link
Contributor Author

snwoods commented Jun 25, 2025

Some functions have multiple string parameters, no labels, and no documentation.

They are all based on the ObserverAPI module, but that's an RPC interface definition using declare , which I don't think I can label, so I don't know what the best way to do this would be? I could add a documentation string for each function like:
(** Creates an observer

  • debug_info
  • uuid
  • name_label
    ...
    *) ?
    But I don't know how helpful that would be, or if I should just point to other documentations of an Observer e.g. Xapi_observer or ocaml/libs/tracing ? I have also labelled the parameters in Server_impl module below and they are the same parameters.

@psafont
Copy link
Member

psafont commented Jun 25, 2025

Documenting them is good, see how parameters are documented in many of the interface files, this one for example: https://github.com/xapi-project/xen-api/blob/master/ocaml/xapi-idl/xen/device_number.mli#L23

@snwoods snwoods force-pushed the private/stevenwo/observer_helpers_mli branch from 663c08d to e424f75 Compare June 25, 2025 12:02
@snwoods snwoods force-pushed the private/stevenwo/observer_helpers_mli branch from e424f75 to fd66e34 Compare July 1, 2025 09:41
@psafont
Copy link
Member

psafont commented Jul 1, 2025

Nothing seems to be running, maybe because of the conflict?

@snwoods snwoods force-pushed the private/stevenwo/observer_helpers_mli branch from fd66e34 to 4927eef Compare July 2, 2025 08:21
@snwoods snwoods added this pull request to the merge queue Jul 2, 2025
Merged via the queue into xapi-project:master with commit 3209c5f Jul 2, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants