Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(105)

Issue 2890013004: Clean up histograms.xml formatting

Created:
3 years, 7 months ago by Ilya Sherman
Modified:
3 years, 7 months ago
CC:
chromium-reviews, asvitkine+watch_chromium.org, Steven Holte
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Clean up histograms.xml formatting * Make it harder to specify the wrong histogram suffix separator by making the attribute required. * Make it harder to omit an enum label by making the attribute required. * While I'm at it, make the name attribute required as well. * Consistently use dashes rather than underscores for naming XML elements. o Affects histogram_suffixes and histogram_suffixes_list. * Drop the redundant type="int" attribute for enums. BUG=724311 TEST=none R=asvitkine@chromium.org

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+2921 lines, -2891 lines) Patch
M tools/metrics/actions/print_style.py View 2 chunks +8 lines, -0 lines 0 comments Download
M tools/metrics/common/pretty_print_xml.py View 2 chunks +19 lines, -4 lines 1 comment Download
M tools/metrics/histograms/enums.xml View 1060 chunks +1874 lines, -1877 lines 0 comments Download
M tools/metrics/histograms/extract_histograms.py View 2 chunks +4 lines, -9 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 363 chunks +998 lines, -993 lines 0 comments Download
M tools/metrics/histograms/print_style.py View 4 chunks +18 lines, -8 lines 0 comments Download

Messages

Total messages: 8 (4 generated)
Ilya Sherman
3 years, 7 months ago (2017-05-18 23:21:24 UTC) #1
Ilya Sherman
FYI, the only changes to enums.xml are dropping type="int" and updating the labels for FtpDataConnectionError. ...
3 years, 7 months ago (2017-05-18 23:24:54 UTC) #4
Alexei Svitkine (slow)
Do we need corresponding internal changes? i.e. in order to process the new tag names.
3 years, 7 months ago (2017-05-19 19:58:30 UTC) #7
Alexei Svitkine (slow)
3 years, 7 months ago (2017-05-19 20:05:28 UTC) #8
LGTM, thanks!

Modulo making the corresponding server-side changes first and ensuring they're
landed and make it to a pipeline build before this lands.

https://codereview.chromium.org/2890013004/diff/1/tools/metrics/common/pretty...
File tools/metrics/common/pretty_print_xml.py (right):

https://codereview.chromium.org/2890013004/diff/1/tools/metrics/common/pretty...
tools/metrics/common/pretty_print_xml.py:215: ', '.join('"{0}"'.format(a) for a
in missing_attributes),
Nit: I'd extract this inner statement into a var above so that this statement is
a bit easier to parse.

Powered by Google App Engine
This is Rietveld 408576698