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