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

Generalize makeEffect to work with more complicated GADTs #17

Conversation

@andremarianiello
Copy link
Contributor

commented Nov 13, 2018

closes #16

Instead of descending down into the types in the constructor looking for free variables (I have a branch that does this) I think it is cleaner to simply reuse the preexisting contexts from the GADT constructor. Thus, if the variable quantification and contexts were correct in the constructor, they should still be correct for the generator function (I think).

I bumped the version, but I don't know if I bumped the right number.

@andremarianiello andremarianiello force-pushed the andremarianiello:make-more-effects branch from 44cecf5 to 677cbaf Nov 13, 2018

@lexi-lambda

This comment has been minimized.

Copy link
Owner

commented Nov 13, 2018

Can you add a test case? Otherwise, this looks good, thanks.

@andremarianiello andremarianiello force-pushed the andremarianiello:make-more-effects branch 2 times, most recently from 6b8bb7f to 1d446c1 Nov 14, 2018

@andremarianiello

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

I added a complex GADT, and I run makeEffect on it inside the test file, but it is not part of a test case per se. Do you have any idea how to create a Testy test case for a TH splice like this? Or is simply compiling the test file with the splice in it good enough?

@lexi-lambda

This comment has been minimized.

Copy link
Owner

commented Nov 15, 2018

Yes, that simple regression test is fine. Thank you for this! Merged as cd7b4c6.

@andremarianiello

This comment has been minimized.

Copy link
Contributor Author

commented Nov 15, 2018

Thanks for the quick release!

@andremarianiello andremarianiello deleted the andremarianiello:make-more-effects branch Nov 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.