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

Fix ordering of floatting doc comments in classes #1970

Merged
merged 1 commit into from Aug 6, 2018

Conversation

Projects
None yet
3 participants
@hhugo
Copy link
Contributor

commented Aug 6, 2018

\cc @lpw25
class_fields and class_sig_fields are built in reverse order.
So should be doc-comments in those constructs.

@nojb

nojb approved these changes Aug 6, 2018

Copy link
Contributor

left a comment

LGTM

@@ -146,6 +146,10 @@ end = struct
(** Ambiguous comment on both titi and toto *)
method toto = tutu ^ "!"

(** floatting 1 *)

This comment has been minimized.

Copy link
@nojb

nojb Aug 6, 2018

Contributor

Here and below: s/floatting/floating

@gasche

This comment has been minimized.

Copy link
Member

commented Aug 6, 2018

I'm not convinced by your fix, why would text_csig and text_cstr return results in reversed order in the first place? Is that desirable, documented anywhere? Shouldn't you be fixing the Docstrings.get_docstrings function instead?

@gasche

This comment has been minimized.

Copy link
Member

commented Aug 6, 2018

P.S.: Which other cases would a global get_docstrings change affect? Do we have testcases for these? Otherwise, could you add some? Are they currently also buggy?

@nojb

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2018

@gasche isn't it the case that text_csig and text_cstr return results in the "right" order and thus they have to be reversed because the rule is assumed to return things in the reversed order?

@gasche

This comment has been minimized.

Copy link
Member

commented Aug 6, 2018

Ah, I see, thanks!

@hhugo hhugo force-pushed the hhugo:doc-com branch from 57ece27 to 662f5d9 Aug 6, 2018

@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2018

I've updated the changelog and squashed commits.

@gasche gasche merged commit 7479ea7 into ocaml:trunk Aug 6, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gasche

This comment has been minimized.

Copy link
Member

commented Aug 6, 2018

(I'll have to rebase #292 against this.)

@hhugo hhugo deleted the hhugo:doc-com branch Aug 6, 2018

@hhugo hhugo referenced this pull request May 14, 2019

Merged

Some small tuning for 4.08 #71

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.