Dan Burkert has posted comments on this change.

Change subject: Add json builder utility
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7387/1/src/kudu/util/jsonbuilder.h
File src/kudu/util/jsonbuilder.h:

Line 1: // Licensed to the Apache Software Foundation (ASF) under one
> FWIW I think it might be helpful if the classname was consistent in that wa
Yah, I'd say keep the class/file name consistent if possible.  I don't have 
much of an opinion on whether EasyJson vs JsonBuilder is better (both seem good 
to me).


Line 102:   scoped_refptr<EasyJsonAllocator> alloc_;
> Ok, I was following the style guide (http://kudu.apache.org/docs/contributi
woops, looks like I was wrong on this one.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie195b06c5ba67af3043d2032c56b90c1a85f641a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

Reply via email to