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

Add module descriptions for all Spar modules #611

Merged
merged 1 commit into from
Feb 7, 2019
Merged

Conversation

neongreen
Copy link
Contributor

I'm very often bitten by "should X go to this module or to some other module?", so I thought spending a bit of time on adding module descriptions would be a good idea.

I would also love it if CI barked at PRs that introduce modules without documentation. Thoughts?

@neongreen neongreen requested a review from fisx February 6, 2019 15:31
@neongreen
Copy link
Contributor Author

(We can check that on CI by running Haddock.)

Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

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

I would also love it if CI barked at PRs that introduce modules without documentation. Thoughts?

I have no strong opinions here. If it doesn't significantly slow PR feedback, sure.

@neongreen neongreen merged commit 91103a5 into develop Feb 7, 2019
@neongreen neongreen deleted the spar-module-docs branch February 7, 2019 14:46
Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

helpful. thanks! sorry for the late review.

@@ -6,6 +6,7 @@
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE PolyKinds #-}

-- | Swagger utilities.
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that means a better place for this module would be Spar.API.Swagger.Util. Oh well.

This was referenced Feb 14, 2019
@neongreen neongreen mentioned this pull request Feb 18, 2019
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.

3 participants