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.

Sign up for GitHub

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

Jump to bottom

Consistent doc comments #296

Merged
merged 2 commits into from
Dec 21, 2019
Merged

Consistent doc comments #296

merged 2 commits into from
Dec 21, 2019

Conversation

CreepySkeleton
Copy link
Collaborator

First commit ensures explicit handling of doc commits and sets strict priority among raw methods and doc comments, see stc/lib.rs diff for details. Closes #173

Second commit introduces no_mangle_docs attributes. Fixes #163

cc @TeXitoi @0ndorio @ssokolow

TeXitoi
TeXitoi requested changes Dec 4, 2019
View reviewed changes
Copy link
Owner

@TeXitoi TeXitoi left a comment

Choose a reason for hiding this comment

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

Great doc and code!

src/lib.rs Outdated Show resolved Hide resolved
tests/doc-comments-help.rs Outdated Show resolved Hide resolved
@CreepySkeleton CreepySkeleton force-pushed the consistent_doc_comments branch 3 times, most recently from f4df576 to 8f7b235 Compare December 4, 2019 13:16
@TeXitoi
Copy link
Owner

TeXitoi commented Dec 4, 2019

Looks like it need a rebase.

@0ndorio
Copy link
Contributor

0ndorio commented Dec 4, 2019

Thanks for all your work. I will spent some time in the evening to read through all of it :)

ssokolow
ssokolow reviewed Dec 4, 2019
View reviewed changes
src/lib.rs Outdated Show resolved Hide resolved
@CreepySkeleton CreepySkeleton force-pushed the consistent_doc_comments branch 4 times, most recently from 144862c to 025a44b Compare December 5, 2019 05:25
@CreepySkeleton
Copy link
Collaborator Author

Closing and opening to retrigger CI build

@CreepySkeleton CreepySkeleton reopened this Dec 5, 2019
@TeXitoi
Copy link
Owner

TeXitoi commented Dec 5, 2019

@CreepySkeleton you should be able to reload a travis job in the travis interface.

@CreepySkeleton
Copy link
Collaborator Author

@TeXitoi While I do have write access to this repo I'm not a member of your travis, I can't do that.
image

If we move to Github actions I'll be able to do that.

@TeXitoi
Copy link
Owner

TeXitoi commented Dec 5, 2019

Strange, they say it works if you have write access to the repository. Are you logged to travis?

@CreepySkeleton
Copy link
Collaborator Author

Err, I wasn't :) Now it works.

@CreepySkeleton CreepySkeleton force-pushed the consistent_doc_comments branch 2 times, most recently from 9157685 to f3505c5 Compare December 5, 2019 14:58
@CreepySkeleton
Copy link
Collaborator Author

OK, this is carefully reworked, rebased and fine-tuned, I'd say it's ready

@CreepySkeleton
Copy link
Collaborator Author

Wait, DO NOT MERGE YET

@CreepySkeleton
Copy link
Collaborator Author

CreepySkeleton commented Dec 5, 2019 •
edited
Loading

OK, it's now mergable. cc @ssokolow Could use a final review

@ssokolow
Copy link

ssokolow commented Dec 5, 2019

I'll see if I can find some time later today to read through the rendered version of the docs and let you know.

(I generally find it significantly harder to get a proper feel for them in their raw form than their rendered form for some reason. If you've ever noticed one of my comments having half a million edits to it, that's why. I forget to use the "Preview" button or, for whatever reason, still only notice something I want to change once I see it in context.)

@CreepySkeleton
Copy link
Collaborator Author

CreepySkeleton commented Dec 5, 2019 •
edited
Loading

still only notice something I want to change once I see it in context.

You're not alone here 😄 It took me about a year to really get used to.

@ssokolow
Copy link

ssokolow commented Dec 6, 2019

By the time things were ready, I'd started to get dozy, but I'll leave the tab open and scrolled to the relevant section so I should be able to get you guys an answer tomorrow morning, as soon as I'm fully alert.

ssokolow
ssokolow suggested changes Dec 6, 2019
View reviewed changes
Copy link

@ssokolow ssokolow left a comment

Choose a reason for hiding this comment

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

There you go. Mostly perfectly understandable ESL (English as a Second Language) mistakes, but I did also tidy up the phrasing in various places.

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@TeXitoi
Copy link
Owner

TeXitoi commented Dec 9, 2019

@CreepySkeleton ping me when that's ready, that I read a second time.

@CreepySkeleton
Copy link
Collaborator Author

Sorry for the radio silence, I had to get back to other stuff I'm affiliated with. I'll get to this PR tomorrow.

@TeXitoi
Copy link
Owner

TeXitoi commented Dec 11, 2019

Take your time.

@CreepySkeleton
Copy link
Collaborator Author

CreepySkeleton commented Dec 13, 2019 •
edited
Loading

cc @TeXitoi @ssokolow the last review time!

I renamed no_mangle_doc to keep_line_breaks so it reflects the behavior better.

TeXitoi
TeXitoi requested changes Dec 16, 2019
View reviewed changes
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@TeXitoi TeXitoi mentioned this pull request Dec 17, 2019
Closed
@CreepySkeleton CreepySkeleton mentioned this pull request Dec 21, 2019
Closed
@CreepySkeleton
Copy link
Collaborator Author

OK, @TeXitoi , please give it a review and let's release it. We've been postponing this long enough already 😟

@TeXitoi TeXitoi merged commit e2270de into master Dec 21, 2019
@CreepySkeleton
Copy link
Collaborator Author

Ugh, I'm writing the changelog that was supposed to go in this PR 😠. Ok let's make a separate one

@TeXitoi
Copy link
Owner

TeXitoi commented Dec 21, 2019

Oups, sorry

@CreepySkeleton CreepySkeleton deleted the consistent_doc_comments branch December 21, 2019 10:49
@TeXitoi
Copy link
Owner

TeXitoi commented Dec 22, 2019

v0.3.6 published

@ducaale ducaale mentioned this pull request Mar 7, 2021
Merged
@epage epage mentioned this pull request Oct 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ssokolow ssokolow ssokolow requested changes

@TeXitoi TeXitoi TeXitoi approved these changes

@0ndorio 0ndorio 0ndorio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

about/long_about and doc-comment precedence is unpredictable Allow preserving newlines
4 participants
@CreepySkeleton @TeXitoi @0ndorio @ssokolow