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

Issue 255510043: Remove misleading comment. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 9 months ago by prabhur
Modified:
10 years, 9 months ago
Reviewers:
fmeawad
CC:
tracing-review_chromium.org
Base URL:
https://github.com/google/trace-viewer@master
Visibility:
Public.

Description

Remove misleading comment. I added this comment in a recent check-in. Thinking about it more, I'm not sure if the array is actually sorted by frequency. If in-fact it was based on frequency, it would be really hard to keep this statement true as there is no mechanism to monitor/measure the frequency & keep the array updated. I suggest removing this comment - to avoid misleading the code-reader. BUG=#1140 R=fmeawad@chromium.org Committed: https://chromium.googlesource.com/external/trace-viewer/+/e2e4136ac775ee4f182727637d922107f65672ec

Patch Set 1 #

Patch Set 2 : Change array to dict. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -4 lines) Patch
M tracing/tracing/extras/importer/trace_event_importer.html View 1 1 chunk +9 lines, -4 lines 0 comments Download

Messages

Total messages: 6
prabhur
Fadi, PTAL. I think traceEvents is more common than the other ones (so sorting it ...
10 years, 9 months ago (2015-07-27 20:07:31 UTC) #1
fmeawad
On 2015/07/27 20:07:31, prabhur wrote: > Fadi, PTAL. > I think traceEvents is more common ...
10 years, 9 months ago (2015-07-27 20:11:21 UTC) #2
fmeawad
On 2015/07/27 20:11:21, fmeawad wrote: > On 2015/07/27 20:07:31, prabhur wrote: > > Fadi, PTAL. ...
10 years, 9 months ago (2015-07-27 20:12:20 UTC) #3
prabhur
PTAL.
10 years, 9 months ago (2015-07-27 20:33:38 UTC) #4
fmeawad
lgtm
10 years, 9 months ago (2015-07-27 20:34:46 UTC) #5
prabhur
10 years, 9 months ago (2015-07-27 20:35:29 UTC) #6
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
e2e4136ac775ee4f182727637d922107f65672ec (presubmit successful).
Sign in to reply to this message.

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