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

Documentation: Added options and negative options to manpages #11647

Conversation

nangahamandine
Copy link
Contributor

@nangahamandine nangahamandine commented Oct 19, 2022

Updated the ocamlopt.1, ocaml.1 and ocamlc.1 files with missing negative options in man directory

@nangahamandine nangahamandine changed the title 6955 synchronize option help in help and man page pt2 Documentation: Added options and negative options to manpages Oct 19, 2022
man/ocaml.1 Outdated
.B \-no-strict\-formats
Do no reject invalid formats that were accepted in legacy format
implementations.
This is the default.
Copy link
Member

Choose a reason for hiding this comment

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

Main options should be sorted by alphabetical order, thus strict-format should be just before strict-sequence.

man/ocaml.1 Outdated
Reject invalid formats that were accepted in legacy format
implementations. You should use this flag to detect and fix
such invalid formats, as they will be rejected by future
OCaml versions.
Copy link
Member

Choose a reason for hiding this comment

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

I would rather have this change in a separate PR because the text in the development branch and in the 5.0 branch needs to be different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you like me to remove the changes made in the ocaml.1 file and change back it to the default?

Copy link
Member

Choose a reason for hiding this comment

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

I was speaking of the -strict-format and -no-strict-format change rather than the changes in ocaml.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh alright. So I'll need to take out the -strict-format and -no-strict-format changes made in all three files for now and later create a separate PR to add the changes, right?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed. Moreover you would need to have one version of the PR for the 5.0 branch and another for the trunk branch because -strict-format is now the default on the trunk branch.

man/ocaml.1 Outdated
@@ -165,7 +165,12 @@ use it once before publishing source code.
.B \-rectypes
Allow arbitrary recursive types during type-checking. By default,
only recursive types where the recursion goes through an object type
are supported.
are supported. Note that once you have created an interface using this
flag, you must use it again for all dependencies.
Copy link
Member

Choose a reason for hiding this comment

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

The REPL does not really create interface that are reused by other users. It seems better to keep this note out of this specific description.

man/ocamlopt.1 Outdated
.B \-inlining-report
Emit .inlining files (one per round of optimisation) showing all of the
inliner’s decisions.

Copy link
Member

Choose a reason for hiding this comment

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

This spurious change is here because you created this branch from the first version of your previous PR, before we moved the location of the flambda section. To remove this change, you can either try to rebase your branch on trunk or remove this change by hand in a commit (that will disappear when this PR is squashed into a single commit).

@nangahamandine
Copy link
Contributor Author

Are the recent changes for this PR okay

man/ocamlopt.1 Outdated
.TP
.B \-rectypes
Allow arbitrary recursive types during type-checking. By default,
only recursive types where the recursion goes through an object type
are supported. Note that once you have created an interface using this
flag, you must use it again for all dependencies.
are supported.
Copy link
Member

Choose a reason for hiding this comment

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

This note should neither be removed from ocamlc.1 and ocamlopt.1 nor added to ocaml.1.

man/ocamlopt.1 Outdated
.B \-inlining-report
Emit .inlining files (one per round of optimisation) showing all of the
inliner's decisions.

Copy link
Member

Choose a reason for hiding this comment

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

This section is now repeated twice at the same location, you should remove it.

Copy link
Member

@Octachron Octachron 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, thanks!
You still need to add a Change entry. I would suggest that you merge the entry with the one from the previous PR:

- #11640, #11647: Add missing options to the man pages:
  flambda commonly-used options, and negative options (`-no-rectypes`, ... ).
  (Amandine Nangah, review by David Allsopp, Florian Angeletti,
  Sébastien Hinderer, and Vincent Laviron)

Copy link
Member

@Octachron Octachron left a comment

Choose a reason for hiding this comment

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

Before merging, there are few formatting typos left that should be fixed.

man/ocaml.1 Outdated Show resolved Hide resolved
man/ocaml.1 Outdated Show resolved Hide resolved
man/ocamlc.1 Outdated Show resolved Hide resolved
man/ocamlopt.1 Outdated Show resolved Hide resolved
man/ocamlopt.1 Outdated Show resolved Hide resolved
@nangahamandine
Copy link
Contributor Author

Can I go ahead to create another PR for the other changes? I was waiting for approval before making other changes

@Octachron
Copy link
Member

You can certainly start working on another PR. You might want to try your hand at a non-documentation issue (maybe #11127 ?).

@nangahamandine
Copy link
Contributor Author

Okay, thanks
I was hoping to complete issue #6955 before moving to another
I've been working on subsets. So if it's okay, may I complete the entire issue first?

@Octachron
Copy link
Member

It would be a better use of your application period to try your hand at more challenging tasks.
(You will be of course very welcome to come back to the issue after the Outreachy application period.)

@nangahamandine
Copy link
Contributor Author

Okay then

@Octachron Octachron merged commit 9f1f71d into ocaml:trunk Oct 21, 2022
@Octachron
Copy link
Member

Merged, thanks again!

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.

None yet

2 participants