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

Update bp_03_run_executables_and_tests.md #2092

Merged
merged 10 commits into from
Oct 1, 2024
Merged

Conversation

F-Loyer
Copy link
Contributor

@F-Loyer F-Loyer commented Feb 24, 2024

Edit: Resolves #1822

This PR address #1822

However, the limitation due to the _build directory lock of dune makes it a little difficult to use the dune build --watch feature.

Copy link
Collaborator

@christinerose christinerose left a comment

Choose a reason for hiding this comment

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

A few suggestions and a question for clarity.

@F-Loyer
Copy link
Contributor Author

F-Loyer commented Feb 26, 2024 via email

Copy link
Contributor Author

@F-Loyer F-Loyer left a comment

Choose a reason for hiding this comment

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

Proposals commited

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 is was missing, thanks @F-Loyer, this greatly helps.

Once this is is complete, we should summarize it there: https://ocaml.org/docs/your-first-program#watch-mode
This PR does a better job at motivating dune build --watch than PR #2064 (which introduced the referred section).

data/tutorials/platform/bp_03_run_executables_and_tests.md Outdated Show resolved Hide resolved
data/tutorials/platform/bp_03_run_executables_and_tests.md Outdated Show resolved Hide resolved

The command `dune build --watch` looks for things that need to be compiled, and afterwards, just waits for a modified file that will trigger the compilation of needed modules.

However, `dune` locks the build directory, then it is not possible to launch two `dune` commands at the same time. The `dune build --watch` has to be stopped (typing Ctrl-C) before launching the application. Or we can launch the application without the help of Dune by typing `_build\default\<executable_path>.exe`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

A remark about launching dune build --watch in the background should be added. For clarity, I believe an item list with the various cases should be added too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok for the background use of dune build --watch, but since this command prints the evolution of the compilation, I find it better to launch it on a dedicated terminal... and work with other terminals.

Do you mean a summary with the different dune commands. dune exec ..., dune build --watch, dune test. There are not a lot of cases.

@@ -43,7 +43,7 @@ You can run it with `dune exec bin/main.exe` or `dune exec my-app`.

The `dune exec <executable_path>.exe` can take some time to compile a project when multiple files are involved. It can be beneficial to have a process that recompiles files as soon as they are changed. Then after the last saved file, it is possible that it remains only a last file to compile or just the linking process of all files and libraries.

The command `dune build --watch` looks for things that need to be compiled, and afterwards, just waits for a modified file that will trigger the compilation of needed modules.
The command `dune build --watch` overwatches for things that need to be compiled. It waits for a file modification that triggers the compilation of needed modules.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think overwatches is word used outside of military tactics. Oversees things? Or just "...watches for things"?

@cuihtlauac
Copy link
Collaborator

cuihtlauac commented Mar 6, 2024

@sabine, here are the edits: https://github.com/ocaml/ocaml.org/tree/dune-watch

@F-Loyer
Copy link
Contributor Author

F-Loyer commented Mar 6, 2024

I read:

Thus it is not possible to launch dune build again, both would be concurrent.

it is not possible, but not needed too since when a file is changed, dune build --watch will be triggered again.

The real issue is with the concurrency of this command and a dune exec.

@christinerose
Copy link
Collaborator

I read:

Thus it is not possible to launch dune build again, both would be concurrent.

it is not possible, but not needed too since when a file is changed, dune build --watch will be triggered again.

The real issue is with the concurrency of this command and a dune exec.

How about: "Thus it's unnecessary to launch dune build again, as both would be concurrent."

@F-Loyer
Copy link
Contributor Author

F-Loyer commented Mar 7, 2024

How about: "Thus it's unnecessary to launch dune build again, as both would be concurrent."

"It is unnecessary to launch dune build again as a building process is triggered when needed. It won't be possible either as both would be concurrent"

@christinerose
Copy link
Collaborator

"It is unnecessary to launch dune build again as a building process is triggered when needed. It won't be possible either as both would be concurrent"

How about:
"It is unnecessary to launch dune build again, as it triggers a building process when necessary. It also won't be possible because both would be concurrent"

@F-Loyer
Copy link
Contributor Author

F-Loyer commented Mar 8, 2024

"It is unnecessary to launch dune build again as a building process is triggered when needed. It won't be possible either as both would be concurrent"

How about: "It is unnecessary to launch dune build again, as it triggers a building process when necessary. It also won't be possible because both would be concurrent"

I may be picky.

"It is unnecessary to launch dune build again, as dune build --watch triggers a building process when necessary. It also won't be possible because both would be concurrent"

or

"It is unnecessary to launch dune build --watch again, as it triggers a building process when necessary. It also won't be possible because both would be concurrent"

@christinerose
Copy link
Collaborator

"It is unnecessary to launch dune build again as a building process is triggered when needed. It won't be possible either as both would be concurrent"

How about: "It is unnecessary to launch dune build again, as it triggers a building process when necessary. It also won't be possible because both would be concurrent"

I may be picky.

"It is unnecessary to launch dune build again, as dune build --watch triggers a building process when necessary. It also won't be possible because both would be concurrent"

or

"It is unnecessary to launch dune build --watch again, as it triggers a building process when necessary. It also won't be possible because both would be concurrent"

Be picky! It's important to get it just right. I'm picky too. 🙂

Either of these work for me, whichever you feel is the most accurate.

@sabine sabine merged commit 5f3f1af into ocaml:main Oct 1, 2024
2 of 3 checks passed
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.

Add Instructions to Watch For File Changes with Dune
4 participants