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

occurences of 'may not' to be more descriptive for issue #83584 #83614

Closed
wants to merge 4 commits into from
Closed

occurences of 'may not' to be more descriptive for issue #83584 #83614

wants to merge 4 commits into from

Conversation

jessehorne
Copy link

@jessehorne jessehorne commented Mar 28, 2021

I did a grep -rl "may not" library/ and went through each instance of may not and made my best guess to what each meant and changed them to be more descriptive. For example, "Humans may not fly." would become "Humans can not fly." or "Humans may not breathe 100% carbon dioxide" to "Humans should not breathe 100% carbon dioxide".

Again, I made my best guess. Please let me know if there are any that are not accurate. I chose not to modify any may or may not's because those are pretty self-explanatory.

Fixes #83584

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 28, 2021
@rust-log-analyzer

This comment has been minimized.

@egilburg
Copy link

Perhaps follow IEFT RFC 2119 for guidance on using these terms? For example, use "should" for best practice advice, and use "must" for things like compiler guarantees or places where failure to do so will cause undefined behavior.

@rust-log-analyzer

This comment has been minimized.

@jessehorne
Copy link
Author

Perhaps follow IEFT RFC 2119 for guidance on using these terms? For example, use "should" for best practice advice, and use "must" for things like compiler guarantees or places where failure to do so will cause undefined behavior.

I agree with RFC 2119 for the most part but I wish it had defined "WILL"/"WILL NOT". Given our use-case, I don't feel like following it exactly would make much sense. For example, if a function is designed to return 1, do you say in the documentation that "this function MUST return '1'", or "this function SHOULD return '1'"? For declaring intent of the implementation, I think "WILL" makes more sense. "MUST" sounds more like you're giving another party information. "WILL" sounds like you're saying "This was designed for that!".

That being said, I didn't use "WILL" at all in this. I mostly just changed from "may not" to either "should not" or "might not" depending on the scenario. While I agree that a standard is helpful when using these terms in documentation, oftentimes it seemed that the use of "may not" in these cases were pretty clear IF you read the previous documentation to gain context. I see the value of this if nothing else to just make it that much clear and potentially save time.

@bors
Copy link
Contributor

bors commented Apr 8, 2021

☔ The latest upstream changes (presumably #84008) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

Ping from triage:
@jessehorne - can you please address the merge conflict?

@rustbot label: -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 26, 2021
@bstrie bstrie added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 12, 2021
@bstrie bstrie added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 2, 2021
@crlf0710 crlf0710 added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 19, 2021
@crlf0710
Copy link
Member

@jessehorne I'm gonna close this due to inactivity. Feel free to reopen or create a new pr when you've got time to work on this again. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert 'may not' to either 'must not' or 'might not' in the standard library documentation
10 participants