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

Explain how to override default entry point #11092

Merged
merged 7 commits into from
Nov 14, 2024

Conversation

benji-sb
Copy link
Contributor

@benji-sb benji-sb commented Nov 4, 2024

This PR adds a page in the "Advanced Topics" section of the doc. This new page explains how to override the default entry point in C and have dune compile everything.

For more context on why this could be useful, you can check the Wildcard expansion on Windows topic on the Ocaml Discourse.

Signed-off-by: Benjamin Sigonneau <benjamin.sigonneau@sandboxaq.com>
@maiste maiste added the docs Documentation improvements label Nov 4, 2024
Copy link
Collaborator

@maiste maiste left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! 👍
We are trying to rely more and more on the Diátaxis framework to improve the Dune documentation. In this situation, I would move this to the How-to guide section in the Dune documentation.

@Leonidas-from-XIV, what is your two cents on this?

doc/advanced/override-default-entrypoint.rst Outdated Show resolved Hide resolved
doc/advanced/override-default-entrypoint.rst Outdated Show resolved Hide resolved
doc/advanced/override-default-entrypoint.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

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

I agree with @maiste, it's definitely more a howto, as it doesn't cover any particular advanced topic about Dune, more about how C stubs work but this has more to do with OCaml than Dune.

Still, we can definitely include it in the documentation.

doc/advanced/override-default-entrypoint.rst Outdated Show resolved Hide resolved
doc/advanced/override-default-entrypoint.rst Outdated Show resolved Hide resolved
@benji-sb
Copy link
Contributor Author

benji-sb commented Nov 7, 2024

I agree with @maiste, it's definitely more a howto, as it doesn't cover any particular advanced topic about Dune, more about how C stubs work but this has more to do with OCaml than Dune.

Still, we can definitely include it in the documentation.

Moved to the howto section, thanks for the suggestion!

Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV 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 it could have a bit of a "this is how the result works" section, otherwise it looks good from my point of view.

@christinerose could you please take a look at the text if it is ok that way?

doc/howto/override-default-entrypoint.rst Show resolved Hide resolved
Copy link
Collaborator

@maiste maiste left a comment

Choose a reason for hiding this comment

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

Thanks, the update looks good! I have just one comment and, otherwise, as @Leonidas-from-XIV said, I let @christinerose do her magic (thanks in advance for having a look at this 🙏 )

Also, you need to update your commit to comply with the DCO (see the information).

doc/howto/override-default-entrypoint.rst Outdated Show resolved Hide resolved
Signed-off-by: Benjamin Sigonneau <benjamin.sigonneau@sandboxaq.com>
Signed-off-by: Benjamin Sigonneau <benjamin.sigonneau@sandboxaq.com>
Signed-off-by: Christine Rose <christinerose@users.noreply.github.com>
Signed-off-by: Benjamin Sigonneau <benjamin.sigonneau@sandboxaq.com>
@Alizter
Copy link
Collaborator

Alizter commented Nov 10, 2024

Just a quick comment: OCaml doesn't have a notion of "entry point" since basically everything is run in order. Therefore calling it the "C Entrypoint" might be better. This might be clearer to new users who might think that OCaml has entry points.

@benji-sb
Copy link
Contributor Author

Just a quick comment: OCaml doesn't have a notion of "entry point" since basically everything is run in order. Therefore calling it the "C Entrypoint" might be better. This might be clearer to new users who might think that OCaml has entry points.

I edited the document accordingly, thanks a lot for bringing up this point!

Signed-off-by: Benjamin Sigonneau <benjamin.sigonneau@sandboxaq.com>
Copy link
Collaborator

@maiste maiste left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@maiste
Copy link
Collaborator

maiste commented Nov 14, 2024

The error in the CI is completely unrelated with the PR (as it just modifies the documentation). I think we can merge it.

@Leonidas-from-XIV Leonidas-from-XIV merged commit 967f566 into ocaml:main Nov 14, 2024
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants