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

Add .cmt* format description #8538

Merged
merged 8 commits into from Apr 6, 2019

Conversation

@Lereena
Copy link
Contributor

commented Mar 23, 2019

Added description of the purpose of .cmt* files in the chapter "Batch compilation (ocamlc)". Issue #7584.

I'm not sure whether to add the same description to the chapter on ocamlopt, since in theory they are pretty much the same.

@Octachron

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2019

I think that we should duplicate the information in both the ocamlc and ocamlopt chapter: readers should be able to read these two chapters independently. But we can do that at the end of the review.

manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
Copy link
Contributor

left a comment

This is a good start but the new paragraph needs a better integration with the current text.

manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
@Lereena

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

I apologize for the long absence.
Added a little introduction. Not sure how correctly understood the remark about the tools.

Copy link
Contributor

left a comment

Do not worry, taking your time is perfectly fine. I have some remark on the global structure to make it a bit more explicit. Nevertheless, this is a step in the right direction:

manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
Copy link
Contributor

left a comment

I have only a handful of remarks left:

manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
Copy link
Contributor

left a comment

One last question, then you can duplicate the new text to the native compiler side and add a Changes entry. It might be nice to add a latex comment (with %comment) to note that this is a duplicate. (At a later date, we might remove the duplication.). Once this is done, I will merge.

manual/manual/cmds/comp.etex Outdated Show resolved Hide resolved
Copy link
Contributor

left a comment

The current state looks good to me. Optionally, you could move your change entry before the PR#8515 one to respect the least issue/pr number ordering.

@Lereena

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2019

Did not pay attention to the order of location. I'll fix it now

@Octachron

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

(You need to rebase on the current trunk, unfortunately changes to Changes often require a fresh version.)

Lereena added 2 commits Apr 6, 2019
@Octachron Octachron merged commit ac94a65 into ocaml:trunk Apr 6, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@Octachron

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2019

Merged, thanks for your time and contribution!

@Lereena Lereena deleted the Lereena:7584 branch Apr 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.