Skip to content

Cabal doctest #33

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

Merged
merged 1 commit into from
Jan 31, 2017
Merged

Cabal doctest #33

merged 1 commit into from
Jan 31, 2017

Conversation

phadej
Copy link
Collaborator

@phadej phadej commented Jan 31, 2017

@phadej phadej force-pushed the cabal-doctest branch 5 times, most recently from ec9e853 to caed9ca Compare January 31, 2017 13:43
Copy link
Collaborator

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

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

Yay yay yay! Thanks a bunch for doing this, @phadej!

I've left some comments inline.

filepath >= 1.3.0.0
base >= 4.5 && <5,
Cabal >= 1.14,
cabal-doctest >= 1 && <1.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does the test-doctests flag interact with this feature? That is, if someone configured this project with -f-test-doctests, would custom-setup also be disabled? (I don't think it's a dealbreaker if we can't make it do that, but I am curious.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IIRC it was a design decision that custom-setup section doesn't support flags / conditionals. But I'm not sure. I'll check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Setup.lhs Outdated
{ buildHook = \pkg lbi hooks flags -> do
generateBuildModule "doctests" flags pkg lbi
buildHook simpleUserHooks pkg lbi hooks flags
}
Copy link
Collaborator

@RyanGlScott RyanGlScott Jan 31, 2017

Choose a reason for hiding this comment

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

This is a little wordier than I would have expected. Couldn't we export something like this from cabal-doctest:

doctestsUserHooks :: UserHooks
doctestsUserHooks testsuiteName = simpleUserHooks
  { buildHook = \pkg lbi hooks flags -> do
     generateBuildModule testsuiteName flags pkg lbi
     buildHook simpleUserHooks pkg lbi hooks flags
  }

And then just do main = defaultMainWithHooks $ doctestUserHooks "doctests"? There's far less boilerplate involved on the commonly taken path (modern Cabal) this way, and you can still override the fields of doctestUserHooks "doctests" if you wish.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, just thought about this omw home. Maybe even also defaultMainWithDoctests

@phadej
Copy link
Collaborator Author

phadej commented Jan 31, 2017

@RyanGlScott could you take another look. If it's ok, I'll release cabal-doctest to Hackage.

@RyanGlScott
Copy link
Collaborator

@phadej, looks great! Please go ahead and upload it to Hackage.

@phadej phadej merged commit 2af110f into ekmett:master Jan 31, 2017
@phadej phadej deleted the cabal-doctest branch January 31, 2017 18:31
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