impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3200 (buffer pool): warn if Status is ignored
Date Fri, 28 Oct 2016 20:23:28 GMT
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 <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message