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

Issue 5824057: Change ClearProductEvent to use RlzValueStore. (Closed)

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

Description

Change ClearProductEvent to use RlzValueStore. TBR=rogerta Committed: https://code.google.com/p/rlz/source/detail?r=83

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -23 lines) Patch
M lib/rlz_value_store.h View 1 chunk +2 lines, -0 lines 0 comments Download
M mac/lib/rlz_value_store_mac.h View 1 chunk +2 lines, -0 lines 0 comments Download
M mac/lib/rlz_value_store_mac.mm View 1 chunk +6 lines, -0 lines 0 comments Download
M win/lib/rlz_lib.cc View 2 chunks +6 lines, -23 lines 2 comments Download
M win/lib/rlz_value_store_registry.h View 1 chunk +2 lines, -0 lines 0 comments Download
M win/lib/rlz_value_store_registry.cc View 1 chunk +17 lines, -0 lines 0 comments Download

Messages

Total messages: 5
thakis
14 years, 7 months ago (2012-03-14 22:07:15 UTC) #1
Roger Tawa (Google)
https://codereview.appspot.com/5824057/diff/1/win/lib/rlz_lib.cc File win/lib/rlz_lib.cc (right): https://codereview.appspot.com/5824057/diff/1/win/lib/rlz_lib.cc#newcode317 win/lib/rlz_lib.cc:317: return store->ClearProductEvent(product, event_name.c_str()); I think you meant event_value and ...
14 years, 7 months ago (2012-03-15 13:49:47 UTC) #2
thakis
https://codereview.appspot.com/5824057/diff/1/win/lib/rlz_lib.cc File win/lib/rlz_lib.cc (right): https://codereview.appspot.com/5824057/diff/1/win/lib/rlz_lib.cc#newcode317 win/lib/rlz_lib.cc:317: return store->ClearProductEvent(product, event_name.c_str()); On 2012/03/15 13:49:47, rogerta wrote: > ...
14 years, 7 months ago (2012-03-15 14:07:38 UTC) #3
Roger Tawa (Google)
OK, but a more important question is why the unit tests did not pick up ...
14 years, 7 months ago (2012-03-15 14:41:52 UTC) #4
thakis
14 years, 7 months ago (2012-03-15 15:04:53 UTC) #5
On 2012/03/15 14:41:52, rogerta wrote:
> OK, but a more important question is why the unit tests did not pick up this
> error?  Can you please take a look and at the tests and maybe fix them for
this
> case?

I mean I fixed this before committing the CL. If you click the "Committed:
https://code.google.com/p/rlz/source/detail?r=83" link and open rlz_lib.cc,
you'll see that that's the correct code.
Sign in to reply to this message.

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