Skip to content

Merge doc-comments-val with doc-comments, new value before-except-val; after becomes after-when-possible#1358

Merged
gpetiot merged 5 commits intoocaml-ppx:masterfrom
gpetiot:doc-comments-v2
May 11, 2020
Merged

Merge doc-comments-val with doc-comments, new value before-except-val; after becomes after-when-possible#1358
gpetiot merged 5 commits intoocaml-ppx:masterfrom
gpetiot:doc-comments-v2

Conversation

@gpetiot
Copy link
Copy Markdown
Collaborator

@gpetiot gpetiot commented May 4, 2020 •
edited
Loading

See #1354 for the discussion that lead to this option.

Currently doing usual tests on big projects, no diff for:

  • mirage
  • irmin
  • index
  • mdx
  • dune
  • owl
  • tezos

@gpetiot gpetiot requested review from Julow and emillon May 4, 2020 09:19
@gpetiot gpetiot requested a review from jberdine May 4, 2020 09:37
@gpetiot gpetiot mentioned this pull request May 4, 2020
Closed
jberdine
jberdine approved these changes May 4, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jberdine jberdine left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

I tested on some of our code and the results are good. There is a lot of churn due to changing the docstrings from after to before on simple module definitions, but that is ok and back to what it used to be.

@jberdine jberdine added this to the 0.14.2 milestone May 4, 2020
@jberdine
Copy link
Copy Markdown
Collaborator

jberdine commented May 5, 2020

Nice! So I think that this is good to go, and then there isn't anything else blocking 0.14.2, right?

@gpetiot
Copy link
Copy Markdown
Collaborator Author

gpetiot commented May 5, 2020 •
edited
Loading

Well I would have liked to have the opinion of @emillon @Julow @samoht on the option before merging it, as we only discussed it the both of us. Just to be sure there is no objection, as the topic was very controversial some weeks ago.

@jberdine
Copy link
Copy Markdown
Collaborator

jberdine commented May 5, 2020

Yes, good point, it would be good to hear from the others.

@samoht
Copy link
Copy Markdown
Contributor

samoht commented May 5, 2020

That looks good to me!

What happens to people using after in their config file? Will they get a warning that the option has been renamed?

@gpetiot
Copy link
Copy Markdown
Collaborator Author

gpetiot commented May 5, 2020 •
edited
Loading

What happens to people using after in their config file? Will they get a warning that the option has been renamed?

I forgot about this. I need to add messages indeed (and same for doc-comments-val for the people that got accustomed to it), but error messages instead of warnings, to not carry the old options until 0.15.

@gpetiot
Copy link
Copy Markdown
Collaborator Author

gpetiot commented May 5, 2020 •
edited
Loading

Here are the warnings now printed:

$ ocamlformat lib/Ast.ml --doc-comments-val=unset
ocamlformat: option `--doc-comments-val': value `unset` has been removed in
             version 0.14.2. The same behavior can be achieved by setting
             `doc-comments` only.

$ ocamlformat lib/Ast.ml --doc-comments-val=before
ocamlformat: option `--doc-comments-val': value `before` has been removed in
             version 0.14.2. If you are using `doc-comments-val=before` in
             combination with `doc-comments=before` then only
             `doc-comments=before` is now required to achive the same
             behavior. If you are using `doc-comments-val=before` in
             combination with `doc-comments=after` this behavior is not
             available anymore.

$ ocamlformat lib/Ast.ml --doc-comments-val=after
ocamlformat: option `--doc-comments-val': value `after` has been removed in
             version 0.14.2. If you are using `doc-comments-val=after` in
             combination with `doc-comments=before` please now use
             `doc-comments=before-except-val`. If you are using
             `doc-comments-val=after` in combination with
             `doc-comments=after` then only `doc-comments=after-when-possible` is now
             required to achieve the same behavior.

$ ocamlformat lib/Ast.ml --doc-comments=after
ocamlformat: option `--doc-comments': value `after` has been removed in
             version 0.14.2. This value has been renamed
             `after-when-possible` to take into account the technical
             limitations of ocamlformat, the behavior is unchanged.

jberdine
jberdine approved these changes May 5, 2020
View reviewed changes
@gpetiot gpetiot merged commit 403e020 into ocaml-ppx:master May 11, 2020
@gpetiot gpetiot deleted the doc-comments-v2 branch May 11, 2020 10:22
@gpetiot gpetiot mentioned this pull request May 11, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jberdine jberdine jberdine approved these changes

@emillon emillon Awaiting requested review from emillon

@Julow Julow Awaiting requested review from Julow

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.14.2

Development

Successfully merging this pull request may close these issues.

4 participants

@gpetiot @jberdine @samoht @facebook-github-bot