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 and cleanup #78

Merged
merged 16 commits into from
Feb 19, 2015
Merged

Documentation and cleanup #78

merged 16 commits into from
Feb 19, 2015

Conversation

olorin
Copy link
Contributor

@olorin olorin commented Feb 18, 2015

Going through and adding missing toplevel Haddock comments, rewriting any that seem terse or unclear, and fixing some minor style/formatting issues as I come across them.

The one nontrivial refactoring made is to remove the (exported) Event type, which is dead code with no remaining use. @christian-marie and I decided that it wouldn't be too terrible to cowboy this into a patch release at this stage of maturity, as the probability of anyone having used it is negligible.

The refactor involves replacing explicit ByteString.appends with mappends; any objections to that?

This is a "breaking change" and so there's a case for making a major
version bump here, but at this stage I think we're pretty safe to cowboy
it - nothing uses or should use the Event type anymore in the
Vaultaire-and-associated codebases, and it's very unlikely that anyone
else would be relying on the presence of a dead sum type at this stage.
@olorin olorin changed the title More inline documentation plus some minor refactoring Documentation and cleanup Feb 19, 2015
@@ -390,12 +392,15 @@ dayMapsFromCeph origin' = do
Right day_map ->
return $ Right (fromIntegral (BS.length contents), day_map)

-- | Ceph object ID of the origin's Simple DayMap.
simpleDayOID :: Origin -> ByteString
simpleDayOID (Origin origin') = "02_" `BS.append` origin' `BS.append` "_simple_days"
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably have used <> here if I had known of it's greatness back then. No need to change now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I missed that one - updated.

olorin added a commit that referenced this pull request Feb 19, 2015
Documentation and cleanup
@olorin olorin merged commit d6a25fe into master Feb 19, 2015
@olorin olorin deleted the dox branch February 19, 2015 03:52
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.

None yet

2 participants