Skip to content

Mdx_top.init can be parameterized by dirs, packages and predicates#244

Merged
gpetiot merged 1 commit intorealworldocaml:masterfrom
gpetiot:parameterize-toplevel-env
Nov 16, 2020
Merged

Mdx_top.init can be parameterized by dirs, packages and predicates#244
gpetiot merged 1 commit intorealworldocaml:masterfrom
gpetiot:parameterize-toplevel-env

Conversation

@gpetiot
Copy link
Copy Markdown
Contributor

@gpetiot gpetiot commented Apr 3, 2020

@NathanReb let me know if this is what you had in mind, before I go further (if yes, I still have to re-architecture the lib to make the whole test function customizable by the dirs).

Copy link
Copy Markdown
Contributor

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

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

I think the most important part here the ~dirs argument as that's the only thing dune will need to provide since the libraries will already be linked in.

I think it makes sense to expose packages and predicates as arguments as well.
Obviously once we've extracted the test logic into a library, they should be arguments, of the entry point for running the tests.

@gpetiot gpetiot marked this pull request as ready for review April 17, 2020 17:17
@gpetiot
Copy link
Copy Markdown
Contributor Author

gpetiot commented Apr 17, 2020

I defined a new library mdx.test, let me know if that's enough for dune.

@gpetiot gpetiot requested a review from NathanReb April 17, 2020 17:18
@gpetiot gpetiot added the no changelog This PR doesn't require a changelog update label Aug 3, 2020
@gpetiot
Copy link
Copy Markdown
Contributor Author

gpetiot commented Nov 13, 2020

I rebased it on master, @voodoos if there is everything you need for the mdx stanza for dune I think we should merge this one, as it's a bit painful to rebase when there is any change in bin/test/main.ml (which happens often)

@voodoos
Copy link
Copy Markdown
Collaborator

voodoos commented Nov 13, 2020

I think everything I need is here, as Nathan said, it is mostly the ~dir parameter which will be useful.

@NathanReb, do you see anything missing ?

Copy link
Copy Markdown
Contributor

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

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

Sure you're right, let's merge this, we can always make changes later if it needs some adjustment. It is already a good change on its own to move the logic from the bin/test/main.ml to the library.

Thanks for working on this and sorry the rebase hassle!

@gpetiot gpetiot merged commit b056a89 into realworldocaml:master Nov 16, 2020
@gpetiot gpetiot deleted the parameterize-toplevel-env branch November 16, 2020 10:27
@voodoos voodoos mentioned this pull request Nov 18, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog This PR doesn't require a changelog update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants