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

Clarify Step Documentation #66583

Merged
merged 1 commit into from
Nov 23, 2019
Merged

Clarify Step Documentation #66583

merged 1 commit into from
Nov 23, 2019

Conversation

Phlosioneer
Copy link
Contributor

While the redesign is in progress (#62886), clarify the purpose of replace_zero and replace_one.

First, "returning itself" is technically impossible due to the function signature of &mut self -> Self. A clone or copy operation must be used. So this is now explicitly stated in the documentation.

Second, the added docs give some guidance about the actual contract around implementation of replace_zero and replace one. Specifically, the only usage is to create a range with no more steps, by setting start to replace_one and end to replace_zero. So the only property that is actually used is replace_one > replace_zero. See #42168 (comment)

The new documentation does not say that is the only contract, and so it should not be considered an api change. It just highlights the most important detail for implementors.

The redesign doesn't seem to be landing any time soon, so this is a stopgap measure to reduce confusion in the meantime.

While the redesign is in progress (rust-lang#62886), clarify the purpose of replace_zero and replace_one.
@rust-highfive
Copy link
Collaborator

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 20, 2019
@Dylan-DPC-zz
Copy link

r? @Dylan-DPC

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 22, 2019

📌 Commit 983cae7 has been approved by Dylan-DPC

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 22, 2019
Centril added a commit to Centril/rust that referenced this pull request Nov 23, 2019
Clarify Step Documentation

While the redesign is in progress (rust-lang#62886), clarify the purpose of replace_zero and replace_one.

First, "returning itself" is technically impossible due to the function signature of &mut self -> Self. A clone or copy operation must be used. So this is now explicitly stated in the documentation.

Second, the added docs give some guidance about the actual contract around implementation of replace_zero and replace one. Specifically, the only usage is to create a range with no more steps, by setting start to replace_one and end to replace_zero. So the only property that is actually used is `replace_one > replace_zero`. See rust-lang#42168 (comment)

The new documentation does not say that is the *only* contract, and so it should not be considered an api change. It just highlights the most important detail for implementors.

The redesign doesn't seem to be landing any time soon, so this is a stopgap measure to reduce confusion in the meantime.
bors added a commit that referenced this pull request Nov 23, 2019
Rollup of 4 pull requests

Successful merges:

 - #65961 (add fn type_name_of_val)
 - #66574 (Update tidy check for error codes testing)
 - #66576 (made gdb pretty-printing more robust when printing uninitialized vec)
 - #66583 (Clarify Step Documentation)

Failed merges:

r? @ghost
@bors bors merged commit 983cae7 into rust-lang:master Nov 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants