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

Feedback on documentation #39

Open
polytypic opened this issue Mar 2, 2024 · 4 comments
Open

Feedback on documentation #39

polytypic opened this issue Mar 2, 2024 · 4 comments
Labels
help wanted Extra attention is needed

Comments

@polytypic
Copy link
Collaborator

polytypic commented Mar 2, 2024

There is always room to improve documentation.

At least for me it can be very difficult to see where the documentation I've written, for code I've designed, isn't necessarily clear enough, because I know too much about the code already.

Feedback on anything that is hard to understand and could perhaps be improved is most welcome.

@polytypic polytypic added the help wanted Extra attention is needed label Mar 2, 2024
@lyrm
Copy link
Contributor

lyrm commented Oct 18, 2024

Here some quick comments on the documentation of the Trigger module. The text is clear, with a lot of useful recommendations.

  • line 256: the and and the end of the line makes it somehow unclear to me. I can't decide if the end of the sentence is missing or if it is meant to be read on OCaml 5, .... and on OCaml 4. In this last case, or will be a more logical connector.
  • line 294: the alert handler does not look good. A : after it would make it more readable for example. Not sure you can do anything about it.
  • Trigger.from_action: I don't get from reading the documentation what this is useful for (but at it is an advanced feature, it may be okay)
  • Line 402: I don't understand the sentence. Is a problem domain a thing or a typo ?

I will probably add a few more comments when I read other modules !

@polytypic
Copy link
Collaborator Author

polytypic commented Oct 19, 2024

Thanks for the feedback!

  • line 256: the and and the end of the line makes it somehow unclear to me. I can't decide if the end of the sentence is missing or if it is meant to be read on OCaml 5, .... and on OCaml 4. In this last case, or will be a more logical connector.

Interesting. The way I think about that is that it is a list of implications or conditional behaviours or rules. IOW, it is an Oxford comma. An implication is always true if the premise is unsatisfied. Likewise you may consider that a conditional behaviour or rule always applies — it just does nothing in case the condition is false. The premises/conditional behaviours in this case are mutually exclusive. Both (or all) of the implications need to be considered or conditional behaviours need to be applied, which justifies the use of "and".

Also consider

let behaviour () =
  if ocaml == 5 then (...); (* , and *)
  if ocaml == 4 then (...)

vs

let behaviour () =
  if ocaml == 5 then (...) else (* or *)
  if ocaml == 4 then (...)

But, more to the point, neither of us is a native English speaker.

Native English speakers: Does the use of "and" seem appropriate?

  • line 294: the alert handler does not look good. A : after it would make it more readable for example. Not sure you can do anything about it.

Yes, I don't think there is much that can be done to make it look better. This is how it looks like in the generated documentation:

Screenshot 2024-10-19 at 15 30 14

  • Trigger.from_action: I don't get from reading the documentation what this is useful for (but at it is an advanced feature, it may be okay)

It is useful when you just want to have an arbitrary callback executed when a trigger you attach to a computation is signaled. I could add a comment to this effect. However, as can be inferred from the documentation, you really need to know what you are doing.

  • Line 402: I don't understand the sentence. Is a problem domain a thing or a typo ?

It is engineering jargon, see e.g. c2 wiki or SE SE

@lyrm
Copy link
Contributor

lyrm commented Oct 22, 2024

Thanks !
About the Computation module: this is also a very understandable module. I like the way it is sorted (by "Interface for...").

  • type packed : this is used in Fiber but not in Computation. Maybe be a link or a hint about what this is used for will be useful.
  • about canceler: this sentence and the next one seem to state the opposite of each other. I got what it meant after reading it a few times (and thanks to the 'usually').
  • about attach_canceler : does attach_canceler do what the documentation of canceler recommend to do with the returned trigger ?
  • canceler and attach_canceler : I don't properly understand how these functions work and what they should be used for, but I am not reading the documentation with the intend of writing a proper scheduler so this probably answer needs/questions that I don't have 😅
  • broken links: structure concurrency link is broken as well as event and promise.

@lyrm
Copy link
Contributor

lyrm commented Oct 22, 2024

Fiber module :

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants