New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 714123 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: Bug



Sign in to add a comment

`git cl format` not removing spaces when removing newline in histograms.xml

Project Member Reported by awdf@chromium.org, Apr 21 2017

Issue description

clang-format produced code that (choose all that apply): 
- Doesn't match Chromium style
- Riles my finely honed stylistic dander
- No sane human would ever choose

It puts it all one line, great but also retains the indentation spaces, not so great.

 <histogram name="Navigation.MainFrameSchemeOTR" enum="NavigationScheme">
   <owner>elawrence@chromium.org</owner>
   <owner>estark@chromium.org</owner>
-  <summary>The scheme of the URL for each main-frame navigation while in
-    incognito.
+  <summary>
+    The scheme of the URL for each main-frame navigation while in     incognito.
   </summary>
 </histogram>


 

Comment 1 by thakis@chromium.org, Apr 21 2017

Owner: asvitk...@chromium.org
Summary: `git cl format not removing spaces when removing newline in histograms.xml (was: clang-format not removing spaces when removing newline in histograms.xml)
clang-format doesn't format xml files. `git cl format` calls different programs

Comment 2 by thakis@chromium.org, Apr 21 2017

for different file types. clang-format handles c++/java/objective-c/js, other programs handle xml/python/gn

Comment 3 by thakis@chromium.org, Apr 21 2017

Summary: `git cl format` not removing spaces when removing newline in histograms.xml (was: `git cl format not removing spaces when removing newline in histograms.xml)

Comment 4 by awdf@chromium.org, Apr 21 2017

Ah my bad, thanks for fixing up the description :)
Components: Internals>Metrics
Status: Available (was: Untriaged)
Project Member

Comment 6 by sheriffbot@chromium.org, Apr 23 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Recharge-Cold
Status: Available (was: Untriaged)
Still relevant.
Status: Assigned (was: Available)
Status: Started (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbbfd4999ab597d65535260f25e30982b08473dc

commit cbbfd4999ab597d65535260f25e30982b08473dc
Author: Alexei Svitkine <asvitkine@chromium.org>
Date: Wed Sep 26 05:45:22 2018

Fix whitespace wrapping in histograms.xml pretty printing.

Previously, the wrapping logic would often result in spans of
whitespace between words that had to be manually fixed up, when
a paragraph of text was re-wrapped by the pretty printer. This
change fixes this, along with a test.

See context bug for an example of the result that was produced
before.

This change applies to all XML formatters and includes the
changes to the XML files based on this new formatting. One
notable difference is that descriptions that used two spaces
between sentences now use one. There was no consistency on
this before and having only one space reduces the size of the
file, so I think it's a positive change.

Bug:  714123 
Change-Id: I619688463ba415a388bccd22d3cf19db19ba109c
Reviewed-on: https://chromium-review.googlesource.com/1244553
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594228}
[modify] https://crrev.com/cbbfd4999ab597d65535260f25e30982b08473dc/tools/metrics/actions/actions.xml
[modify] https://crrev.com/cbbfd4999ab597d65535260f25e30982b08473dc/tools/metrics/common/pretty_print_xml.py
[modify] https://crrev.com/cbbfd4999ab597d65535260f25e30982b08473dc/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/cbbfd4999ab597d65535260f25e30982b08473dc/tools/metrics/histograms/histograms.xml
[add] https://crrev.com/cbbfd4999ab597d65535260f25e30982b08473dc/tools/metrics/histograms/pretty_print_test.py
[modify] https://crrev.com/cbbfd4999ab597d65535260f25e30982b08473dc/tools/metrics/rappor/rappor.xml
[modify] https://crrev.com/cbbfd4999ab597d65535260f25e30982b08473dc/tools/metrics/ukm/ukm.xml

Should be fixed. One thing to follow up is to have the new test run on the bots.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c5ce638e36966b872f26e2df86116388e06fd51

commit 5c5ce638e36966b872f26e2df86116388e06fd51
Author: Alexei Svitkine <asvitkine@chromium.org>
Date: Thu Oct 04 23:30:41 2018

Make histograms/pretty_print_test.py run on the bots.

This test was added via:
https://chromium-review.googlesource.com/1244553

This is a follow-up CL to add it to metrics_python_tests so
that it will run on the bots.

Renames Python files to avoid name collisions between
different scripts, so that they can run together as part of
metrics_python_tests. Also fixes up the rappor model test
and adds it to the suite.

Bug:  714123 
Change-Id: I466584493f7090decdb8b21217347a905b3e6391
Reviewed-on: https://chromium-review.googlesource.com/c/1255596
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596921}
[modify] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/BUILD.gn
[rename] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/actions/actions_print_style.py
[modify] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/actions/extract_actions.py
[modify] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/histograms/generate_expired_histograms_array_unittest.py
[rename] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/histograms/histograms_print_style.py
[modify] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/histograms/pretty_print.py
[modify] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/metrics_python_tests.py
[modify] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/rappor/pretty_print.py
[rename] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/rappor/rappor_model.py
[rename] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/rappor/rappor_model_test.py
[modify] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/ukm/pretty_print.py
[rename] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/ukm/ukm_model.py
[rename] https://crrev.com/5c5ce638e36966b872f26e2df86116388e06fd51/tools/metrics/ukm/ukm_model_test.py

Status: Fixed (was: Started)
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9cf546abc899f13e372202d3e7b97374015ff9cc

commit 9cf546abc899f13e372202d3e7b97374015ff9cc
Author: Daniel Cheng <dcheng@chromium.org>
Date: Fri Oct 05 00:04:39 2018

Revert "Make histograms/pretty_print_test.py run on the bots."

This reverts commit 5c5ce638e36966b872f26e2df86116388e06fd51.

Reason for revert: broke the build

Original change's description:
> Make histograms/pretty_print_test.py run on the bots.
> 
> This test was added via:
> https://chromium-review.googlesource.com/1244553
> 
> This is a follow-up CL to add it to metrics_python_tests so
> that it will run on the bots.
> 
> Renames Python files to avoid name collisions between
> different scripts, so that they can run together as part of
> metrics_python_tests. Also fixes up the rappor model test
> and adds it to the suite.
> 
> Bug:  714123 
> Change-Id: I466584493f7090decdb8b21217347a905b3e6391
> Reviewed-on: https://chromium-review.googlesource.com/c/1255596
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596921}

TBR=dpranke@chromium.org,asvitkine@chromium.org,holte@chromium.org

Change-Id: Iebebe68f9c4c4da11e6e79fc14b9853bea737e5c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  714123 
Reviewed-on: https://chromium-review.googlesource.com/c/1263434
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596931}
[modify] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/BUILD.gn
[modify] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/actions/extract_actions.py
[rename] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/actions/print_style.py
[modify] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/histograms/generate_expired_histograms_array_unittest.py
[modify] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/histograms/pretty_print.py
[rename] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/histograms/print_style.py
[modify] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/metrics_python_tests.py
[rename] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/rappor/model.py
[modify] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/rappor/pretty_print.py
[rename] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/rappor/pretty_print_test.py
[rename] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/ukm/model.py
[modify] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/ukm/pretty_print.py
[rename] https://crrev.com/9cf546abc899f13e372202d3e7b97374015ff9cc/tools/metrics/ukm/pretty_print_test.py

https://chromium-review.googlesource.com/c/chromium/src/+/1263237 was submitted to fix the issue, hence the revert of the revert.
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3

commit 980759fbb321d960dc4a52b6fb3ffb9f8228a3d3
Author: Alexei Svitkine <asvitkine@chromium.org>
Date: Fri Oct 05 00:10:03 2018

Reland "Make histograms/pretty_print_test.py run on the bots."

This reverts commit 9cf546abc899f13e372202d3e7b97374015ff9cc.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Revert "Make histograms/pretty_print_test.py run on the bots."
> 
> This reverts commit 5c5ce638e36966b872f26e2df86116388e06fd51.
> 
> Reason for revert: broke the build
> 
> Original change's description:
> > Make histograms/pretty_print_test.py run on the bots.
> > 
> > This test was added via:
> > https://chromium-review.googlesource.com/1244553
> > 
> > This is a follow-up CL to add it to metrics_python_tests so
> > that it will run on the bots.
> > 
> > Renames Python files to avoid name collisions between
> > different scripts, so that they can run together as part of
> > metrics_python_tests. Also fixes up the rappor model test
> > and adds it to the suite.
> > 
> > Bug:  714123 
> > Change-Id: I466584493f7090decdb8b21217347a905b3e6391
> > Reviewed-on: https://chromium-review.googlesource.com/c/1255596
> > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#596921}
> 
> TBR=dpranke@chromium.org,asvitkine@chromium.org,holte@chromium.org
> 
> Change-Id: Iebebe68f9c4c4da11e6e79fc14b9853bea737e5c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  714123 
> Reviewed-on: https://chromium-review.googlesource.com/c/1263434
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596931}

TBR=dcheng@chromium.org,dpranke@chromium.org,asvitkine@chromium.org,holte@chromium.org

Change-Id: Ie026170296321cd0cbd4a6fd57506f88388ba295
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  714123 
Reviewed-on: https://chromium-review.googlesource.com/c/1263238
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596936}
[modify] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/BUILD.gn
[rename] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/actions/actions_print_style.py
[modify] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/actions/extract_actions.py
[modify] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/histograms/generate_expired_histograms_array_unittest.py
[rename] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/histograms/histograms_print_style.py
[modify] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/histograms/pretty_print.py
[modify] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/metrics_python_tests.py
[modify] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/rappor/pretty_print.py
[rename] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/rappor/rappor_model.py
[rename] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/rappor/rappor_model_test.py
[modify] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/ukm/pretty_print.py
[rename] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/ukm/ukm_model.py
[rename] https://crrev.com/980759fbb321d960dc4a52b6fb3ffb9f8228a3d3/tools/metrics/ukm/ukm_model_test.py

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/729e8925aa417e161db2e703d15343cd0b95c90f

commit 729e8925aa417e161db2e703d15343cd0b95c90f
Author: Sam McNally <sammc@chromium.org>
Date: Fri Oct 05 02:08:44 2018

Fix missed print_style renames.

Rename the remaining uses of print_style to histograms_print_style left
from https://crrev.com/596921.

Bug:  714123 
Change-Id: Ia5caab102a50bbee2c28573808ad592d48c6cc1b
Tbr: asvitkine@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1263636
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596961}
[modify] https://crrev.com/729e8925aa417e161db2e703d15343cd0b95c90f/tools/metrics/histograms/update_editor_commands.py
[modify] https://crrev.com/729e8925aa417e161db2e703d15343cd0b95c90f/tools/metrics/histograms/update_histogram_enum.py
[modify] https://crrev.com/729e8925aa417e161db2e703d15343cd0b95c90f/tools/metrics/histograms/update_policies.py

Sign in to add a comment