Dan Hecht has posted comments on this change.

Change subject: IMPALA-3200 (buffer pool): warn if Status is ignored
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4878/2//COMMIT_MSG
Commit Message:

PS2, Line 7: IMPALA-3200 (buffer pool)
IMPALA-2615


http://gerrit.cloudera.org:8080/#/c/4878/2/be/src/bufferpool/buffer-allocator.h
File be/src/bufferpool/buffer-allocator.h:

PS2, Line 38: MUST_USE(Status)
This is going to get pretty noisy if we start tagging all return types like 
this.  In the future (perhaps after we fix the warnings over time), will it be 
feasible to mark the Status type itself with this attribute?

And until then, an alternate syntax worth considering is to put the attribute 
after the function decl, like how Kudu does it, e.g.:

Status SetFlushMode(FlushMode m) WARN_UNUSED_RESULT;

To my eyes that's a bit easier to read since it's not so central to the 
declaration. But I don't feel too strongly about it if the consensus prefers 
the current syntax.


-- 
To view, visit http://gerrit.cloudera.org:8080/4878
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c26e7532b5f2c7fe167accc73179e8b72b192bc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbap...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to