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

Documentation Overhaul #27

Merged
merged 14 commits into from
Sep 25, 2020
Merged

Documentation Overhaul #27

merged 14 commits into from
Sep 25, 2020

Conversation

fosskers
Copy link
Collaborator

@fosskers fosskers commented Sep 20, 2020

The streaming ecosystem is wonderful, but its public-facing documentation contains much content from its early days of experimentation. These explanations tend to be verbose and lack practical examples.

This PR reworks this library's README and other relevant documentation to be more user friendly.

Docstring coverage is now also 100%.

@fosskers fosskers changed the title README Overhaul Documentation Overhaul Sep 20, 2020
@fosskers fosskers marked this pull request as ready for review September 20, 2020 21:39
Data/ByteString/Streaming/Char8.hs Outdated Show resolved Hide resolved
Comment on lines +336 to +337
-- | 'fold_' keeps the return value of the left-folded bytestring. Useful for
-- simultaneous folds over a segmented bytestream.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what "keeps" means here. It rather seems like the "return value" (r) is actually ignored. Perhaps you meant "transforms" rather than "keeps", since the (x -> b) argument is applied to the fold result.

Also "simultaneous" doesn't quite sound right either, I see this as rather "sequential". So this is just a fold over all the bytes, with the result then transformed via a final mapping?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to trust the original wording, assuming that people who actually use the foldl library would understand this.

Data/ByteString/Streaming/Char8.hs Show resolved Hide resolved
Data/ByteString/Streaming/Char8.hs Show resolved Hide resolved
Data/ByteString/Streaming/Char8.hs Outdated Show resolved Hide resolved
Data/ByteString/Streaming/Internal.hs Show resolved Hide resolved
Data/ByteString/Streaming/Internal.hs Show resolved Hide resolved
Data/ByteString/Streaming/Internal.hs Outdated Show resolved Hide resolved
Data/ByteString/Streaming/Internal.hs Show resolved Hide resolved
Data/ByteString/Streaming/Internal.hs Outdated Show resolved Hide resolved
Co-authored-by: Viktor Dukhovni <viktor1ghub@dukhovni.org>
@@ -713,8 +695,7 @@ putStrLn bs = hPut IO.stdout (snoc bs '\n')
-- , count
-- , count'

{-| This will read positive or negative Ints that require 18 or fewer characters.
-}
-- | This will read positive or negative Ints that require 18 or fewer characters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know whether you subscribe to the haskell-cafe list, but in a post there today, I mentioned in passing that the readInt in this module is also egregiously slow. I got a 5x performance improvement when I wrote my own, and it entirely avoids the bogus 18-char limit, which is semantically dubious (wrong!) and cause overly long strings of decimals to parsed as multiple ints in successive calls to readInt for example.

So (likely a separate PR), we should replace readInt with something like my (not yet 100% polished) version...

Data/ByteString/Streaming/Internal.hs Show resolved Hide resolved
Data/ByteString/Streaming/Internal.hs Show resolved Hide resolved
Data/ByteString/Streaming/Internal.hs Show resolved Hide resolved
Data/ByteString/Streaming/Internal.hs Show resolved Hide resolved
@fosskers
Copy link
Collaborator Author

Backticks do work to link symbols in Haddock.

@vdukhovni
Copy link
Contributor

Backticks do work to link symbols in Haddock.

I did not know that, I've only seen single-quotes for that. Thanks for the note. I guess you can use whichever you prefer then, sorry about asking for these to be changed...

@fosskers
Copy link
Collaborator Author

No prob! I've using backticks for years so I was totally on autopilot when I added them all here. Pushing all your other suggestions up now...

@fosskers fosskers merged commit d3ccabd into master Sep 25, 2020
@fosskers fosskers deleted the colin/readme-revamp branch September 25, 2020 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants