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

Create Haddocks from entity documentation comments #1503

Merged
merged 10 commits into from
Sep 18, 2023

Conversation

blujupiter32
Copy link
Contributor

@blujupiter32 blujupiter32 commented Jun 27, 2023

This PR adds a flag to MkPersistSettings which enables Haddock generation from entity documentation comments. The default is False to avoid any unexpected performance impacts from upgrading persistent.

Fixes #1462.


Before submitting your PR, check that you've:

  • Documented new APIs with Haddock markup
  • Added @since declarations to the Haddock
    • Changes to the documentation assume that this feature will be made available in persistent version 2.14.6.0.
  • Ran stylish-haskell on any changed files.
    • stylish-haskell fails on the #if-#else-#endif construct in the import list in TH.hs as all three preprocessor directives are ignored, resulting in two identifier lists for Language.Haskell.TH.Lib.
  • Adhered to the code style (see the .editorconfig file for details)

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Bumped the version number if there isn't an (unreleased) on the Changelog
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

@parsonsmatt
Copy link
Collaborator

This is fantastic, thanks!

Can you clarify the limitatiion around documenting fields?

Can you post the problematic test? That smells like a GHC bug to me, and might be worth reporting upstream.

For a test, I'd be happy to see a rendered Haddock that contains the docs - a screenshout would be fine.

@blujupiter32
Copy link
Contributor Author

On closer inspection, the limitation only applies to withDecDoc, so I've added Haddock support for fields using putDoc. As for the test, I've added a spec module in blujupiter32#5 which fails with the error. Building the Haddock documentation for persistent from this branch should show an additional Test.HaddockExample module with entries generated from the doc comments.

persistent/persistent.cabal Outdated Show resolved Hide resolved
persistent/test/Database/Persist/TH/EntityHaddockSpec.hs Outdated Show resolved Hide resolved
Previously, field Haddocks would only be processed if a documentation
comment was present on the model. This commit also uses putDoc for
both field and model Haddocks, removing the withDecDoc import.
@blujupiter32 blujupiter32 marked this pull request as ready for review June 28, 2023 17:15
@blujupiter32
Copy link
Contributor Author

Regarding version bumping, would this change require a minor version bump (to 2.15.0.0) as mpsEntityHaddocks has been added to the public API?

Also, would it be worth addressing #1296 and #1297 (and merging #1505) before releasing this change so that documentation comments are syntactically equivalent to the format supported by Haddock? Alternatively, these issues could be fixed and released in subsequent patch versions if frequent releases aren't a problem.

@parsonsmatt
Copy link
Collaborator

This would be a minor version bump, which put be 2.14.6.0, since PVP has two major versions.

I think I'd be fine releasing this separately from changing the way the comment syntax works.

@blujupiter32
Copy link
Contributor Author

e2420bc conflicts with 0bc8942, so would you prefer rebasing onto master or merging master into the feature branch? PRs seem to be squash-merged so I don't think it would make much difference either way.

@parsonsmatt
Copy link
Collaborator

You can do either; I generally find that merges make viewing diffs slightly easier on GitHub since I can see "changes since last review," but you're right that it'll get squash/merged into master anyway.

@parsonsmatt parsonsmatt added this to the 2.14.6 milestone Jun 29, 2023
@parsonsmatt parsonsmatt merged commit 98ee544 into yesodweb:master Sep 18, 2023
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.

Doc comments should be included in Haddock, optionally
2 participants