lgtm w/ nits
https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h File include/v8-tracing.h (right): https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h#newcode32 include/v8-tracing.h:32: // These values must be in sync with macro values in trace_event.h in nit: s/trace_event.h/trace_log.h https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h#newcode41 include/v8-tracing.h:41: }; Should we add a placeholder for ETW_EXPORT 1 << 3 here? Or comment that it's used? https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h#newcode59 include/v8-tracing.h:59: }; DISALLOW_COPY_AND_ASSIGN? https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h#newcode60 include/v8-tracing.h:60: } // namespace v8 nit: blank line above https://codereview.chromium.org/988893003/diff/240001/src/tracing/event-tracer.cc File src/tracing/event-tracer.cc (right): https://codereview.chromium.org/988893003/diff/240001/src/tracing/event-tracer.cc#newcode44 src/tracing/event-tracer.cc:44: } nit: no {}'s on single line bodies https://codereview.chromium.org/988893003/diff/240001/src/tracing/trace-event.h File src/tracing/trace-event.h (right): https://codereview.chromium.org/988893003/diff/240001/src/tracing/trace-event.h#newcode60 src/tracing/trace-event.h:60: Should this be UNIMPLEMENTED()? https://codereview.chromium.org/988893003/diff/240001/src/tracing/trace-event.h#newcode111 src/tracing/trace-event.h:111: // FIXME crbug? https://codereview.chromium.org/988893003/ -- -- 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.
