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

(doc) add more info about multiple files #1942

Merged
merged 8 commits into from Jan 22, 2024

Conversation

heathhenley
Copy link
Contributor

@heathhenley heathhenley commented Jan 14, 2024

Attempting to address #1856 as I learn Ocaml - is something like this example reasonable to add to clear up how to add more files / modules in a library?

Other things:

  • if the module has the same name as the library as with hello in the example, is this some type of special situation?
  • I followed the linked thread (https://discuss.ocaml.org/t/using-modules-15-characters-long-now/) for how add expose modules from other files within the module that shares a name with the library, but in terms of the interface file, it was a guess. What is the correct what to expose those in the interface file?

Sorry if this is way off I don't have really any experience with OCaml yet.

@cuihtlauac
Copy link
Collaborator

Thanks @heathhenley.

I'm sorry I forgot to link to the issue number in my own fix to #1856. It is in PR #1923. You couldn't see I was working on that too.

However, your PR addresses the issue in a slightly different manner, it is possible to combine them. Can you rebase this PR after #1923 gets merged?

@heathhenley
Copy link
Contributor Author

Oops, yes no problem I'll rebase on your update after you merge and see if mine still add anything useful.

@cuihtlauac
Copy link
Collaborator

We've merged #1923. You can rebase. This will allow shortening your patch since module reexports are no longer needed.

My file world.ml probably needs to be renamed en.ml

@heathhenley heathhenley force-pushed the dev_modules_in_multiple_files branch 3 times, most recently from 2aafe84 to 90af21b Compare January 18, 2024 23:40
@heathhenley heathhenley marked this pull request as ready for review January 18, 2024 23:42
@heathhenley
Copy link
Contributor Author

@cuihtlauac, I've just rebased and reconciled with your updates. Let me know what you think!

Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

Reviewed to line 213, more to come

heathhenley and others added 3 commits January 19, 2024 08:16
Co-authored-by: Cuihtlauac Alvarado <cuihtlauac@users.noreply.github.com>
@heathhenley
Copy link
Contributor Author

Applied / made the requested changes so far, let me know what else you need. 👍

Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

This looks good to me. @sabine, @christinerose do you want to have a look?

@cuihtlauac cuihtlauac merged commit 706623b into ocaml:main Jan 22, 2024
2 of 3 checks passed
@heathhenley heathhenley deleted the dev_modules_in_multiple_files branch January 22, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants