Import Rewrote Set Tutorial from V2 PR #948
Conversation
|
@christinerose 's suggestions look good to me. Is there a way to quickly approve/merge in all of them? |
I've committed them. |
|
@NebuPookins: Thanks for the time you spent rewriting this tutorial. On behalf of the ocaml.org maintenance team, I must apologise to you, your PR got lost when migrating from V2 to V3. I'm really sorry about that. Despite those odd circumstances, it's a pleasure to see you reacting to the resurrection of your PR. I'd like to move forward with your PR. I believe the points raised by @Octachron in the initial discussion can be addressed, what is your feeling about that? |
There were a few that weren’t yet committed, so I refreshed them but still approved so it can be merged when you’re ready. |
|
I must have messed up something at some point. Firstly, files which shouldn't have been there were included, and then I accidentally closed the PR. Let's hope this text isn't bad luck :-) As far as I can see the greyed suggestions, which continue to appear seem to be bogus. I see them in the text. |
|
Hi @cuihtlauac no need to apologize. The unfortunate reality is I've moved on from OCaml for right now, so these topics are no longer fresh in my memory, so I don't think I would do a good job of updating the tutorial. In my (possibly biased) opinion, I think the version I submitted is still an improvement over the previous content, so my recommendation would be to merge it in as-is, and then have someone else (possibly @Octachron ) do another pass of it to address the concerns they raised. |
|
Thanks @NebuPookins, for your kind answer. It is perfectly understandable you've moved on to other matters. I'm happy we have your permission to reuse that text and modify it. I'll probably do. Thanks again |
data/tutorials/ds_02_set.md
Outdated
| which you can assign the name `SS` (short for "String Set"). Note: Be sure to | ||
| pay attention to the case; you need to type `Set.Make(String)` and not | ||
| `Set.Make(string)`. The reason behind this is explained in the | ||
| "Technical Details" section at the bottom. |
There was a problem hiding this comment.
This can be replaced by a link to the section, so we can remove "at the bottom".
data/tutorials/ds_02_set.md
Outdated
| signatures of many of the functions defined in this module. | ||
|
|
||
| For example, the `add` function has the signature `elt -> t -> t`, which means | ||
| that it expects an element (a String), and a set of strings, and will return to you |
There was a problem hiding this comment.
a String.t
Also, I would just say "will return", I find it weird to add "to you" but maybe that's just me.
data/tutorials/ds_02_set.md
Outdated
| function which matches this signature: `cardinal : t -> int`. If you're | ||
| not familiar with the word "cardinal," you can look it up on Wikipedia | ||
| and notice that it basically refers to the size of sets, so this reinforces | ||
| the idea that this is exactly the function we want. |
There was a problem hiding this comment.
I would just give the definition here without alluding to wikipedia, to make it more self-contained. Same for singletons and Purely Functional Data Structure.
It gives a very unformal tone to this page, which is also the case for "Data structures > Map", but not for the others. Related to #1154.
@NebuPookins rewrote the set tutorial for [V2](https://github.com/ocaml/v2.ocaml.org) in 2021. The PR was neither merged nor rejected: ocaml/v2.ocaml.org#1596
Co-authored-by: Christine Rose <[email protected]>
As pointed by @Octachron
|
There are several examples meant for the toplevel, but they don't contain the prompt li: 28, 87, 95, 103, 152, 181, 191, and 208 |
@NebuPookins rewrote the set tutorial for V2 in 2021. The PR was neither merged nor rejected:
ocaml/v2.ocaml.org#1596