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

🛐 Worship the OCaml module system #94

Merged
merged 15 commits into from
Jul 9, 2022
Merged

🛐 Worship the OCaml module system #94

merged 15 commits into from
Jul 9, 2022

Conversation

jonsterling
Copy link
Contributor

@jonsterling jonsterling commented Jul 8, 2022

This pull request contains two changes:

  1. Replace the handler records with modules. I found (experimentally) while hacking algaett that it is a lot less painful to program the handlers with modules than it is to write out the records. Features like include and open are useful, and I would not discount reducing the stress of being several parenthesis/brace delimiter levels into a gnarly piece of code.

  2. I have refactored the modules so that the main signatures are declared (and documented) just once. This removes a lot of duplication that is annoying when refactoring code. It also means that if documentation is generated for the private/internal modules, the documentation comments will appear for these as well. Sometimes we wish to expose a different public interface than the private interface (but I didn't notice that in this library); that too can be incorporated, through judicious use of include.

Let me know if you have any feedback about these changes, or if you don't like it for some reason. Thanks!

src/ModifierSigs.ml Outdated Show resolved Hide resolved
src/ScopeSigs.ml Outdated Show resolved Hide resolved
@jonsterling jonsterling requested a review from favonia July 9, 2022 09:30
@favonia
Copy link
Collaborator

favonia commented Jul 9, 2022

Just wanted to add that, under the records=modules Thought, these two approaches should be isomorphic. I hope we can fix this problem at least in the languages/tools we created. :-)

@favonia favonia merged commit 2585043 into main Jul 9, 2022
@favonia favonia deleted the use-modules branch July 9, 2022 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