-
-
Notifications
You must be signed in to change notification settings - Fork 417
Conversation
Thanks for your pull request and interest in making D better, @edi33416! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + druntime#2498" |
Why does this need to be moved for phobos deduplication? |
At the very least it should be imported by cc'ing @TurkeyMan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving to core.internal
is good as it's the place for public, yet intended for internal use, functions.
A "simpler" way would be to make emplaceRef
public and leave it in core.lifetime
, but I don't think we should advertise emplaceRef
as part of the public API, or at least not yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Imho it's not going to go anywhere anyhow (like |
I chose local imports against top level ones so a user won't pay the import cost if he doesn't use |
Move
emplaceRef
tocore.internal.lifetime
so we can remove the code duplication from Phobos.emplaceRef
is intentionally non-ddoc'ed