Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(126)

Issue 5757052: Tidying

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 3 months ago by Ben Laurie (Google)
Modified:
14 years, 3 months ago
Reviewers:
ekasper, ben
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : Fix comment #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -37 lines) Patch
M src/merkletree/LogDB.h View 2 chunks +9 lines, -14 lines 0 comments Download
M src/merkletree/Makefile View 1 chunk +2 lines, -0 lines 0 comments Download
M src/merkletree/TreeLogger.h View 1 2 chunks +3 lines, -5 lines 2 comments Download
M src/merkletree/TreeLogger.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/merkletree/TreeLoggerTest.cc View 4 chunks +12 lines, -17 lines 3 comments Download

Messages

Total messages: 5
Ben Laurie (Google)
14 years, 3 months ago (2012-03-06 18:04:10 UTC) #1
Ben Laurie (Google)
14 years, 3 months ago (2012-03-06 18:07:20 UTC) #2
ekasper
LGTM modulo propagating the changes all the way up. http://codereview.appspot.com/5757052/diff/3001/src/merkletree/TreeLogger.h File src/merkletree/TreeLogger.h (right): http://codereview.appspot.com/5757052/diff/3001/src/merkletree/TreeLogger.h#newcode51 src/merkletree/TreeLogger.h:51: ...
14 years, 3 months ago (2012-03-06 18:45:26 UTC) #3
ben
http://codereview.appspot.com/5757052/diff/3001/src/merkletree/TreeLogger.h File src/merkletree/TreeLogger.h (right): http://codereview.appspot.com/5757052/diff/3001/src/merkletree/TreeLogger.h#newcode51 src/merkletree/TreeLogger.h:51: size_t PendingLogSize() const { return db_->PendingLogSize(); } On 2012/03/06 ...
14 years, 3 months ago (2012-03-07 13:57:35 UTC) #4
ekasper
14 years, 3 months ago (2012-03-07 14:48:07 UTC) #5
http://codereview.appspot.com/5757052/diff/3001/src/merkletree/TreeLoggerTest.cc
File src/merkletree/TreeLoggerTest.cc (right):

http://codereview.appspot.com/5757052/diff/3001/src/merkletree/TreeLoggerTest...
src/merkletree/TreeLoggerTest.cc:52: static const char *nibble =
"0123456789abcdef";
On 2012/03/07 13:57:35, ben wrote:
> On 2012/03/06 18:45:26, ekasper wrote:
> > Why make these static and not anything else?
> 
> They seemed obviously local. 
> 
> > 
> > In general, tests already live in unnamed namespace... (sigh, here we go
> again)
> 
> I have yet to understand why this is considered better, or even as good.
Looking
> at this declaration, how can I tell it is in an unnamed namespace?

Don't know (perhaps because you can also put classes in an unnamed namespace?)
but I guess we should be consistent, and having both makes no sense. So if you
tidy this, tidy everything, everywhere :)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b