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

Issue 5902045: Set chromium_code for rlz. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years ago by thakis
Modified:
12 years ago
CC:
rlz-codereviews_googlegroups.com
Base URL:
https://rlz.googlecode.com/svn/trunk
Visibility:
Public.

Description

Set chromium_code for rlz. This does two things: 1.) It enables filename rules. Use this to simplify rlz.gyp. 2.) It enables more warnings. Fix them. BUG=none TEST=none

Patch Set 1 #

Total comments: 3

Patch Set 2 : rebase #

Patch Set 3 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -37 lines) Patch
M lib/crc8_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M lib/financial_ping_test.cc View 1 chunk +0 lines, -1 line 0 comments Download
M lib/machine_id.cc View 1 chunk +1 line, -1 line 0 comments Download
M lib/rlz_lib.cc View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M lib/string_utils_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M mac/lib/rlz_value_store_mac.mm View 1 1 chunk +0 lines, -1 line 0 comments Download
M rlz.gyp View 1 2 3 chunks +19 lines, -30 lines 0 comments Download

Messages

Total messages: 4
thakis
As discussed. I haven't tried this on windows yet, will do soon.
12 years ago (2012-03-23 22:22:34 UTC) #1
Roger Tawa (Google)
lgtm Thanks Nico, this is great! One nit below. https://codereview.appspot.com/5902045/diff/1/mac/lib/rlz_value_store_mac.mm File mac/lib/rlz_value_store_mac.mm (left): https://codereview.appspot.com/5902045/diff/1/mac/lib/rlz_value_store_mac.mm#oldcode307 mac/lib/rlz_value_store_mac.mm:307: ...
12 years ago (2012-03-24 00:04:46 UTC) #2
thakis
Thanks! https://codereview.appspot.com/5902045/diff/1/mac/lib/rlz_value_store_mac.mm File mac/lib/rlz_value_store_mac.mm (left): https://codereview.appspot.com/5902045/diff/1/mac/lib/rlz_value_store_mac.mm#oldcode307 mac/lib/rlz_value_store_mac.mm:307: NSString* folder = CreateRlzDirectory(); On 2012/03/24 00:04:46, rogerta ...
12 years ago (2012-03-24 00:06:29 UTC) #3
Roger Tawa (Google)
12 years ago (2012-03-24 00:24:18 UTC) #4
https://codereview.appspot.com/5902045/diff/1/mac/lib/rlz_value_store_mac.mm
File mac/lib/rlz_value_store_mac.mm (left):

https://codereview.appspot.com/5902045/diff/1/mac/lib/rlz_value_store_mac.mm#...
mac/lib/rlz_value_store_mac.mm:307: NSString* folder = CreateRlzDirectory();
On 2012/03/24 00:06:29, thakis wrote:
> On 2012/03/24 00:04:46, rogerta wrote:
> > maybe the variable is not used, but are you sure the call does not need to
be
> > made?
> 
> Yes, RlzPlistFilename() (2 lines below) calls CreateRlzDirectory() too.

Cool.
Sign in to reply to this message.

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