https://codereview.chromium.org/596533002/diff/100001/test/cctest/test-sampler-api.cc
File test/cctest/test-sampler-api.cc (right):

https://codereview.chromium.org/596533002/diff/100001/test/cctest/test-sampler-api.cc#newcode10
test/cctest/test-sampler-api.cc:10: #include "src/simulator.h"
On 2014/09/24 13:44:07, yurys wrote:
Do we still need this include?

Done.

https://codereview.chromium.org/596533002/diff/100001/test/cctest/test-sampler-api.cc#newcode42
test/cctest/test-sampler-api.cc:42: DCHECK_EQ(NULL, instance_);
On 2014/09/24 13:44:07, yurys wrote:
I'd rather inline this check and singlton implementation into
SamplingTestHelper

Done.

https://codereview.chromium.org/596533002/diff/100001/test/cctest/test-sampler-api.cc#newcode80
test/cctest/test-sampler-api.cc:80: Sample& get_sample() { return
sample_; }
On 2014/09/24 13:44:07, yurys wrote:
style: no  get_ prefix on simple getters.

Done.

https://codereview.chromium.org/596533002/diff/100001/test/cctest/test-sampler-api.cc#newcode87
test/cctest/test-sampler-api.cc:87: void DoCollectSample() {
On 2014/09/24 13:44:07, yurys wrote:
This should be private

Done.

https://codereview.chromium.org/596533002/diff/100001/test/cctest/test-sampler-api.cc#newcode105
test/cctest/test-sampler-api.cc:105: switch (event->type) {
On 2014/09/24 13:44:07, yurys wrote:
We should ignore code events after the sample is taken.

Nice catch! Done.

https://codereview.chromium.org/596533002/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to