David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8127 )
Change subject: WIP: Pull StatusOr<T> and related tests from protobuf ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8127/1/src/kudu/util/statusor.h File src/kudu/util/statusor.h: http://gerrit.cloudera.org:8080/#/c/8127/1/src/kudu/util/statusor.h@94 PS1, Line 94: class StatusOr { > Going to bike shed a bit (sorry). I think the name is kind of unwieldy, an Let's bikeshed away :) It's not like this is urgent. In a vacuum I'd maybe lean more twoards Result, but I like StatusOr<T> here because of the parallelism with the currently existing Status. Say I'm adding a new method to tablet (which already has a bunch of methods that return Status) to acquire row locks. With StatusOr it would be something like: StatusOr<WriteTransactionState*> AcquireRowLocks(); While with result it would be something like: Result<WriteTransactionState*> AcquireRowLocks(); I think StatusOr fits naturally better with the current methods that already return status. Also it's slightly more explicit about the fact that it can fail, at the cost of being more "ugly". http://gerrit.cloudera.org:8080/#/c/8127/1/src/kudu/util/statusor.h@98 PS1, Line 98: UNKNOWN > do we have such a thing? no, none of the comments were updated as of yet (I'm using Incomplete()) -- To view, visit http://gerrit.cloudera.org:8080/8127 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If2d86a3769da733456956fb892de6d679a011a63 Gerrit-Change-Number: 8127 Gerrit-PatchSet: 1 Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Fri, 22 Sep 2017 22:33:21 +0000 Gerrit-HasComments: Yes
