-
Notifications
You must be signed in to change notification settings - Fork 99
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
Create ReadTheDocs project for unordered-containers. #249
base: master
Are you sure you want to change the base?
Create ReadTheDocs project for unordered-containers. #249
Conversation
Friendly ping @treeowl :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this looks great! A few comments below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for that @m-renaud! it looks good.
@m-renaud why the change? |
Oops, that was actually a mistake, I was looking at the wrong section of the docs. Undone. |
Oh wait, you're right, |
It'll exist soon! Great. We can add more docs when it's merged. As is, with your changes here, we should be good to merge this. |
@sjakobi your eyes would be appreciated. |
Thanks for doing this, @m-renaud! :) I think this kind of introductory documentation is very important. Nevertheless, I wish that this was integrated in the haddocks:
Would you consider converting this into one or several tutorial modules in the style of |
Thanks for the feedback @sjakobi! I briefly considered putting the tutorial in the haddocks but opted for RST + ReadTheDocs for a few reasons:
To speak to the specific points you've raised:
Having a sentence at/near the top of the entrypoint into the docs referring to the (external) docs doesn't seem much different than having a sentence which refers to the Also, SEO should take care of indexing it properly as well, for example here's a search for
That's a fair point, I don't think RST is that much different than Markdown, but its still slightly different.
I actually think its the opposite, with ReadTheDocs each page has a direct link to where you can edit the documentation right from the rendered docs, which I think actually lowers the barrier for making edits. Additionally, as I mentioned above, changing haddocks requires a new release of the package for the changes to take affect which may dissuade external contributors.
Eh, I can buy that, but also a specific directory named
If you're adding/changing a function in a module, you still need to know to update the "intro docs", regardless of if that lives at I'd love to hear your thoughts on my points above! In any case, if you feel strongly that this should live in a |
No worries, it's always good to get more opinions :) And yeah I can do that, I'm presuming there a plan to cut a new release of the package soon so that the documented |
I should hope so 😄. I'm going to go through the merges since last release and put together a criteria for the next release. It should include the space leak fixes, the new combinators, and also most of these new doc changes. |
You make good points @m-renaud! I'm very happy that you care and have thought about this stuff so deeply! 👍 I'll respond to some of your arguments:
Haddock's default style was changed with GHC-8.10 (or 8.8 already?), bringing it closer to Stackage's style.
I wouldn't mind making releases for documentation improvements, and will do so, if @treeowl gives me upload permissions. Having a separate process for documentation changes, with separate permissions also makes things more complicated.
Haddock markup is admittedly tricky. At this point I'm sufficiently experienced with it that it's probably easier for me than writing RST though. In my experience RST has its pitfalls too though, and the subtle differences from Markdown don't exactly help. Since this documentation is targeted at beginners, who are unlikely to know either Haddock or RST, I believe the ideal markup language to invite contributions would actually be Markdown!
You're not wrong, but I also don't believe that the effect on compile times is large enough that this matters. In fact, having these docs included in the haddocks, means that people can simply build them locally, e.g. with the classic
This is an intriguing idea! IMHO the better way to improve the reputation of Haskell library docs would be to improve the existing ones though. Even if you can pull off writing and publishing "intro docs" for a large swath of libraries, people will still encounter the (bad) old ones and might be put off.
If we'd come to a point where searching for "unordered-containers docs" would give results for both the ReadTheDocs site and Hackage, I think that could cause confusion too…
Not a bad point. I'm under the impression though that your intro docs for
I didn't mean an issue with discoverability here, but rather that when there are two separate "documentation trees", that might lead to some decision paralysis on where best to document certain things.
I feel bad to ask you for more after all the work you've already put into this! Nevertheless I think it's worth giving this a spin. Maybe just try it with a single page. If we still end up preferring the ReadTheDocs version, I'm definitely in favour of publishing it! |
Oh great!
The docs auto-rebuild whenever they're changed, so pretty minimal overhead (can also be done manually with a single button push).
Eh, I don't think its uncommon for intro/beginner tutorials being hosted separately from API docs, as long as there's cross links I don't think it would be much of an issue.
It actually has at least 5 unique contributions (haskell/containers@8b0b600, haskell/containers@58ca823, haskell/containers@0f61449, haskell/containers@3a343a3, haskell/containers@17494c9)! The files were all moved into a different directory which apparently didn't maintain "history" but they still show up in "blame".
I migrated some of the content to a
I'm also a bit of a Haddock noob, so its possible that there are ways to make the above better. |
Open questions:
|
Many thanks for giving this a spin, @m-renaud! I'm having some trouble building your branch though:
I'm quite mystified where that error comes from. CI is green too… How did you build the haddocks locally? |
I can reproduce the build failure in da5ac39, the parent commit of your branch. Rebasing on top of |
- Change copyright/author to 'unordered-containers maintainers' - Change warning about HashMap.!? - Remove incomplete Serialization sections, todo in follow-up.
c977e9f
to
a331c1c
Compare
Yup, looks like an old base CL, rebased and re-pushed commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've taken a first look and added a few inline comments and suggestions that might help clear things up.
I see that you're struggling with the markup a bit. I'd suggest that you look at some existing tutorial modules like Pipes.Tutorial
or Dhall.Tutorial
for examples.
I think it's worth putting a bit more effort into this, so we get a somewhat fair comparison between Haddocks and ReadTheDocs.
If you have more questions, I'm happy to help!
Does that sound OK for you?
How can I get subheaders to show up in the TOC?
I think you have to use the top-level *
, **
etc. headings for that.
Any tips for improving navigation between tutorial modules?
AFAIK there isn't much support for anything but module name references, e.g. "Tutorial.HashSet"
.
Support for anchors will improve with GHC 8.12 though: haskell/haddock#1179
Tutorial/HashSet.hs
Outdated
order to keep the changes you need to assign it to a new variable. For example: | ||
|
||
@ | ||
let s1 = 'HashSet.fromList' ["a", "b"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The easiest way to get qualified names is probably this:
let s1 = 'HashSet.fromList' ["a", "b"] | |
let s1 = "Data.HashSet".'HashSet.fromList' ["a", "b"] |
I think this will only work with the full module name though.
There are also haddock options for this, but I'm not sure how reliable they are:
https://haskell-haddock.readthedocs.io/en/latest/invoking.html#cmdoption-qual
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that's a little ugly, but it works.
Tutorial/HashSet.hs
Outdated
@ | ||
import qualified 'Data.HashSet' as HashSet | ||
|
||
let dataStructures = 'HashSet.fromList' ["HashSet", "HashMap", "Graph"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is in a @
-block which supports (some?) Haddock markup, the strings are interpreted as module names! You can either
-
switch to
>
instead of@
-
use strings that aren't valid module names
-
escape the
"
s :Suggested changelet dataStructures = 'HashSet.fromList' ["HashSet", "HashMap", "Graph"] let dataStructures = 'HashSet.fromList' [\"HashSet", \"HashMap", \"Graph"]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Thanks for the suggestions @sjakobi, I am indeed not experienced with Haddock markup 🙃. I think I'll have some time tonight to fix the markup in the existing docs and add a bit more content. |
Oh look! Haddock uses RST and ReadTheDocs for its user-facing docs 😜 |
BTW another IMHO pretty good-looking tutorial module is http://hackage.haskell.org/package/turtle-1.5.19/docs/Turtle-Tutorial.html. Of course, it's also by Gabriel Gonzalez! |
Added some more content for comparison. Outstanding issues:
Do you think it would be helpful to solicit some more opinions? |
I'm getting a build failure again:
|
FWIW, I've started to make a list of tutorial modules here: https://github.com/sjakobi/awesome-haskell-tutorial-modules 😄 |
Oops, forgot to git add the new file. |
@sjakobi to throw another option out there: Have the examples and walkthroughs in the module documentation as is done in Rust (see https://doc.rust-lang.org/1.5.0/std/collections/struct.HashMap.html for example), instead of a separate Tutorial module. Then we could improve the inline haddocks for the the individual functions to contain examples and better explanations, and then re-arrange the sections a bit. wdyt? Aside: It may not be a great fit for this type of content, but the modules should probably have an example usage in them anyways. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm submitting this review mostly to publish my two inline comments that I had written earlier.
I'm mostly in favour of the inline example version in #267 now.
My current impression of tutorial modules is that they are mostly used to introduce some kind of DSL. The u-c API is quite simple though – no real need for a full tutorial.
@ | ||
import qualified Data.HashSet as HashSet | ||
|
||
let s1 = HashSet.'HashSet.fromList' ["a", "b"] | ||
let s2 = HashSet.'HashSet.delete' "a" s1 | ||
print s1 | ||
> HashSet.'HashSet.fromList' ["a","b"] | ||
print s2 | ||
> HashSet.'HashSet.fromList' ["b"] | ||
@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This style seems rather unusual and also doesn't fit with the way most GHCi sessions look. The usual doctest
-style seems clearer to me:
@ | |
import qualified Data.HashSet as HashSet | |
let s1 = HashSet.'HashSet.fromList' ["a", "b"] | |
let s2 = HashSet.'HashSet.delete' "a" s1 | |
print s1 | |
> HashSet.'HashSet.fromList' ["a","b"] | |
print s2 | |
> HashSet.'HashSet.fromList' ["b"] | |
@ | |
@ | |
>>> import qualified Data.HashSet as HashSet | |
>>> HashSet.'HashSet.fromList' ["a", "b"] | |
fromList ["a","b"] | |
>>> HashSet.'HashSet.delete' "a" s1 | |
fromList ["b"] | |
@ |
HashSet.fromList ["base", "containers", "QuickCheck"] | ||
> fromList [,"containers","base","QuickCheck"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HashSet.fromList ["base", "containers", "QuickCheck"] | |
> fromList [,"containers","base","QuickCheck"] | |
HashSet.fromList ["base", "containers", \"QuickCheck"] | |
> fromList ["containers","base",\"QuickCheck"] |
These docs walk a newcomer through the unordered-containers package.
Content includes:
Rendered docs can be found at https://m-renaud-haskell-unordered-containers.readthedocs.io/en/m-renaud-docs-user-guide/
Once approved these will be published at https://haskell-unordered-containers.readthedocs.io.
This resolves #173.
/cc @treeowl for review