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

Core.Time documentation is confusing #129

Open
Isweet opened this issue Apr 19, 2019 · 2 comments
Open

Core.Time documentation is confusing #129

Isweet opened this issue Apr 19, 2019 · 2 comments
Labels
forwarded-to-js-devs This report has been forwarded to Jane Street's internal review system.

Comments

@Isweet
Copy link

Isweet commented Apr 19, 2019

I'm not sure how much you can do about this, since the documentation is automatically generated and the Time module is complicated (as documented here).

That being said, the Core_time_float documentation is very cluttered, which makes it difficult to tell which hyperlinked module is the actual module that is exposed by Core.

For example, there are 3 top-level modules called Span. Two of these link to the empty module. One of them links to the exposed Span module.

I'm personally pretty comfortable with OCaml, but even I had a hard time navigating this documentation. I would imagine that a beginner would have even more trouble making sense of this.

I think this should be a priority for any new documentation efforts, since it is a fundamental module when using other libraries (such as Async, which is why I was looking into Span). Even just adding some prose to help people navigate to the correct modules would be great.

I'm playing with Async in my free time, and if I make enough progress I'll be happy to submit a PR with some Time documentation.

@yminsky
Copy link

yminsky commented Apr 19, 2019

Yeah, it's a sad state of affairs. The work that @jonludlum is doing on odoc (which we're funding via OCaml Labs) should help make it possible to improve the docs in a serious way. But there's work we can probably do on time in particular to make it less horrible in the meantime.

You can look at this page to get a sense of the problem. You'll see that a fair amount of effort has gone into creating documentation. But it doesn't flow through the current tools in a reasonable way. For example, this bit, which is the opening stanza of the docs for Time_ns:

(** Time represented as an [Int63.t] number of nanoseconds since the epoch.
    See {!Core.Time_ns} for important user documentation.
    Internally, arithmetic is not overflow-checked.  Instead, overflows are silently
    ignored, as for [int] arithmetic.  Conversions may (or may not) raise if prior
    arithmetic operations overflowed. *)

just doesn't show up on the corresponding doc page. This is just a straight-up bug in the doc generation.

@Isweet
Copy link
Author

Isweet commented Apr 19, 2019

Thanks for following up!

I guess I should walk back any claims that things are poorly documented, but rather just hard to navigate. Once I found the Time.Span module that had all the documentation (and wasn't just an empty module with type t -- still not sure why this happens), it was fantastic.

If odoc will render it properly, just adding some hyperlinked modules to the top of Core.Time (i.e. Core__.Core_time_float) could be an easy and impactful short-term change.

Something like:

(** Time exposes the submodules:
- {{!Time.Span}[Span]}
- {{!Time.Ofday}[Ofday]}
...

*)

where !Time.Span is the appropriately qualified name of the module (maybe Core__.Core_time_float.Time.Span?).

@github-iron github-iron added the forwarded-to-js-devs This report has been forwarded to Jane Street's internal review system. label Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
forwarded-to-js-devs This report has been forwarded to Jane Street's internal review system.
Projects
None yet
Development

No branches or pull requests

3 participants