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

Use third form in method's docs + small code refactoring #2027

Merged
merged 11 commits into from
Jan 24, 2020

Conversation

olegnn
Copy link
Contributor

@olegnn olegnn commented Dec 24, 2019

Use third person form in method's docs everywhere. Added some missing comments. Small refactoring. Other docs fixes.

@olegnn olegnn changed the title Use third form in methods comments + small code refactoring Use third form in method's docs + small code refactoring Dec 24, 2019
Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

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

Thanks. This is a great change. I skimmed and the goal of the doc changes is good.

It would be easier to get this PR merged if the code changes are submitted in a separate PR. I didn't look at all of them yet (ran out of time), but I found one (BytesCodec(())) where the private field exists explicitly to support adding fields in the future w/o a breaking change.

Anyway, if you are able to split out the code changes separately, that would make things easier. Otherwise, I will try to review more later.

tokio-util/src/codec/bytes_codec.rs Outdated Show resolved Hide resolved
@olegnn
Copy link
Contributor Author

olegnn commented Jan 17, 2020

@carllerche I fixed breaking changes.
Could you take a look? Otherwise I will close this PR because it gets more and more conflicts with master.

@dekellum
Copy link
Contributor

dekellum commented Jan 22, 2020

I would certainly agree that consistency for the tokio project is helpful, but if you don't mind me (an interested 3rd party) asking: what's the rationale for 3rd person form?

I personally favor 1st person as in:

/// Return `Bar` if available.
fn foo() -> Option<Bar>;

One could interpret the sentence as in:

I (the fn foo!) return Bar if available

Maybe that is too colloquial or folksy for some? I favor brevity in this regard, I think.

Edit: And as proof of my preference: #2129 (which might also conflict with this):

https://docs.rs/tokio/0.2.10/tokio/task/fn.yield_now.html

@olegnn olegnn closed this Jan 24, 2020
@carllerche
Copy link
Member

Sorry for the delay. Let me look again. I'll deal w/w conflicts

@carllerche carllerche reopened this Jan 24, 2020
Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

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

Thanks for sticking with this 👍 I reverted the code changes even if they were purely stylistic. With a doc PR of this size, it makes it much easier to split up code changes from docs. If you wish to open a PR w/ the code changes, we can work on that next 👍

@carllerche carllerche merged commit f9ddb93 into tokio-rs:master Jan 24, 2020
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

3 participants