-
Notifications
You must be signed in to change notification settings - Fork 14.1k
Rename DropGuard::into_inner to DropGuard::dismiss
#148589
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
Conversation
Also changes it from a static method to an instance method.
|
rustbot has assigned @Mark-Simulacrum. Use |
This comment has been minimized.
This comment has been minimized.
|
Some alternative options (since I did not see any yet):
|
|
In the tracking issue, ACP, impl PR, and libs-api meeting the following options were brought up as well:
That leaves us with: |
|
I assume libs-api nominated is appropriate? |
|
Yes, that's great - thank you! |
|
@yoshuawuyts If a guard guards a thing and you dismiss them, what happens with the thing? I guess it depends on how big/heavy the thing is. I think I like
|
|
I like
|
|
We discussed this in today's @rust-lang/libs-api meeting. One of our goals in the naming was to not emphasize "getting the value back" the way that Given that, the main names we focused on were We ended up not getting a consensus around |
That's great; thanks for discussing this in the meeting. This matches the Linux kernel API and is also what this PR has implemented. I believe this PR should now be good to merge! |
|
Shall we merge this since the consensus has been achieved? I'm going to adopt this feature in our database project 1 and would try to avoid further rename breakings. Footnotes |
|
@bors r+ rollup |
Rollup of 6 pull requests Successful merges: - #148256 (remove support for `typeof`) - #148589 (Rename `DropGuard::into_inner` to `DropGuard::dismiss`) - #149001 (Fix false positive of "multiple different versions of crate X in the dependency graph") - #149334 (fix ICE: rustdoc: const parameter types cannot be generic #149288) - #149345 (Deeply normalize param env in `compare_impl_item` if using the next solver) - #149367 (Tidying up UI tests [4/N]) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #148589 - yoshuawuyts:DropGuard-dismiss, r=joshtriplett Rename `DropGuard::into_inner` to `DropGuard::dismiss` Tracking issue: #144426 One of the open questions blocking the stabilization of `DropGuard` is what to name the associated method that prevents the destructor from running, and returns the captured value. This method is currently called `into_inner`, but most people (including myself) feel like this would benefit from a method that calls more attention to itself. This PR proposes naming this method `dismiss`, after the Linux kernel's [`ScopeGuard::dismiss`](https://rust.docs.kernel.org/kernel/types/struct.ScopeGuard.html#method.dismiss). Which crucially does not evoke images of violence or weaponry the way alternatives such as "disarm" or "defuse" do. And personally I enjoy the visual metaphor of "dismissing a guard" (e.g. a person keeping watch over something) - a job well done, they're free to go. This PR also changes the signature from an static method to an instance method. This also matches the Linux kernel's API, and seems alright since `dismiss` is not nearly as ubiquitous as `into_inner`. This makes it more convenient to use, with a much lower risk of conflicting. Though in the rare case there might be ambiguity, the explicit notation is available as a fallback. ```rust let x = DropGuard::into_inner(guard); // ← current let x = guard.dismiss(); // ← proposed
Tracking issue: #144426
One of the open questions blocking the stabilization of
DropGuardis what to name the associated method that prevents the destructor from running, and returns the captured value. This method is currently calledinto_inner, but most people (including myself) feel like this would benefit from a method that calls more attention to itself.This PR proposes naming this method
dismiss, after the Linux kernel'sScopeGuard::dismiss. Which crucially does not evoke images of violence or weaponry the way alternatives such as "disarm" or "defuse" do. And personally I enjoy the visual metaphor of "dismissing a guard" (e.g. a person keeping watch over something) - a job well done, they're free to go.This PR also changes the signature from an static method to an instance method. This also matches the Linux kernel's API, and seems alright since
dismissis not nearly as ubiquitous asinto_inner. This makes it more convenient to use, with a much lower risk of conflicting. Though in the rare case there might be ambiguity, the explicit notation is available as a fallback.