Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1506943002:
style: Allow std::move, the std::copy counterpart. (Closed)
Descriptionstyle: Allow std::move, the std::copy counterpart.
This patch allows the algorithm std::move usage.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
R=thakis@chromium.org
Committed: https://crrev.com/d53c32105101633b798b12d9c5c9cfac3c8d3509
Cr-Commit-Position: refs/heads/master@{#363579}
Patch Set 1 #Patch Set 2 : #Messages
Total messages: 18 (9 generated)
Description was changed from ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage and adds a sample use. ========== to ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage and adds a sample use. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
Description was changed from ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage and adds a sample use. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage and adds a sample use. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel R=thakis@chromium.org ==========
vmpstr@chromium.org changed reviewers: + thakis@chromium.org
Please take a look.
So I don't see a reason for not allowing this, but I also don't think I see a reason for using it in practice. In the file you linked to, the rhs isn't any shorter than the lhs and to people who don't know this move() overload it's much much more confusing :-/
On 2015/12/07 18:36:28, Nico wrote: > So I don't see a reason for not allowing this, but I also don't think I see a > reason for using it in practice. In the file you linked to, the rhs isn't any > shorter than the lhs and to people who don't know this move() overload it's much > much more confusing :-/ That's a fair point. I've only really added usage because that's what we should do to ensure trybots pass. To be honest, I wouldn't write move in that case either. How do you suggest we proceed? Just allow without usage? :) Or put this on a backburner for now?
Allow without usage and put some discouraging text in the note section ("This is
allowed, but almost always not using this function is less code and much less
confusing to people who don't know this move() overload", "You are allowed to
use this but if you do your coworkers _will_ make fun of you", ...), I suppose
:-/
On 2015/12/07 18:54:33, Nico wrote:
> Allow without usage and put some discouraging text in the note section ("This
is
> allowed, but almost always not using this function is less code and much less
> confusing to people who don't know this move() overload", "You are allowed to
> use this but if you do your coworkers _will_ make fun of you", ...), I suppose
> :-/
Sounds good. PTAL.
lgtm with fixed cl description (which advertises a use of this still)
Description was changed from ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage and adds a sample use. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel R=thakis@chromium.org ========== to ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel R=thakis@chromium.org ==========
The CQ bit was checked by vmpstr@chromium.org
The CQ bit was unchecked by vmpstr@chromium.org
The CQ bit was checked by vmpstr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1506943002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1506943002/20001
Message was sent while issue was closed.
Description was changed from ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel R=thakis@chromium.org ========== to ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel R=thakis@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel R=thakis@chromium.org ========== to ========== style: Allow std::move, the std::copy counterpart. This patch allows the algorithm std::move usage. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel R=thakis@chromium.org Committed: https://crrev.com/d53c32105101633b798b12d9c5c9cfac3c8d3509 Cr-Commit-Position: refs/heads/master@{#363579} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/d53c32105101633b798b12d9c5c9cfac3c8d3509 Cr-Commit-Position: refs/heads/master@{#363579} |
