mirage / bechamel

Agnostic benchmark in OCaml (proof-of-concept)
MIT License
44 stars 15 forks source link

Doc fixes #32

Closed edwintorok closed 2 years ago

edwintorok commented 2 years ago

I've noticed some typos and fixed them. I've also found some paragraphs/docstrings in Bechamel.mli a bit difficult to follow, having to reread some sentences and "parse" them more carefully to understand their meaning. The master branch is already better than 0.2.0 in this regard, but I think the sentences can be made easier to read by rearranging their order or simplifying them in some cases.

One docstring that was difficult to read was this:

So,
    [one analyze time { Benchmark.stat; lr; kde; }] where [analyze] is
    initialized with [run] {i predictor} wants to estimate actual
    {i run}-[time] (or execution time) value

I think some commas or other separators would've helped clarify what the subject of 'wants' is, but I think this is easier to understand if written like this where we get to the point sooner and define all the pieces at the end. Also using a noun fits better:

    So,
   [one analysis time { Benchmark.stat; lr; kde; }]
     wants to estimate actual {i run}-[time] (or execution time) value,
     where [analysis] is initialized with [run] {i predictor}.

I'm not a native speaker, so the changes here may not be entirely correct, or optimal. I tried to separate each change into its own commit with a small justification of why I think the change improves the documentation. It'd be ideal if a native speaker could review these changes.

dinosaure commented 2 years ago

Thanks, I'm not a native speaker too and my English speaking is probably too close to my French speaking. I will try to ask someone to check what you did but I generally agree with your remark 🙂 .

dinosaure commented 2 years ago

Thanks! I think it's really fine, I will probably cut a release as soon as possible.