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

refactor: extract modules from Install_rules #7590

Closed
wants to merge 2 commits into from

Conversation

emillon
Copy link
Collaborator

@emillon emillon commented Apr 20, 2023

Package_paths and Stanzas_to_entries are made top level in dune_rules, and the modules related to installation are moved to a install subdirectory.

@emillon emillon requested a review from rgrinberg April 20, 2023 12:50
emillon added 2 commits April 20, 2023 15:10
`Package_paths` and `Stanzas_to_entries` are made top level in
`dune_rules`.

Signed-off-by: Etienne Millon <me@emillon.org>
Signed-off-by: Etienne Millon <me@emillon.org>
@emillon emillon force-pushed the stanzas-to-entries branch from 1a63375 to a2994a1 Compare April 20, 2023 13:10
@rgrinberg
Copy link
Member

It's a bit of a shame to make so many additional functions public. Can we stick to exporting only what is being used?

@emillon
Copy link
Collaborator Author

emillon commented Apr 20, 2023

It's a bit of a shame to make so many additional functions public. Can we stick to exporting only what is being used?

What module are you thinking of specifically?
The Stanzas_to_entries part came up in #7480 - it seemed that it made sense to put it in its own module.
For the packag paths module, I agree it's a bit random. Do you mean that some of these functions could be moved to where they're used in Install_rules?

@rgrinberg
Copy link
Member

Yes, Package_paths is the offender. It has a lot of functions that I wouldn't want anyone using.

@emillon emillon closed this May 26, 2023
@emillon emillon deleted the stanzas-to-entries branch May 26, 2023 12:00
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