Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2910983002:
Improve and fix counter texts (Closed)
DescriptionImprove and fix counter texts
Change all CBD counter texts to upper case first letter.
Replace cookie counter text for consistency with other counters.
BUG=681523
Review-Url: https://codereview.chromium.org/2910983002
Cr-Original-Original-Commit-Position: refs/heads/master@{#475344}
Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538cec8090b5cdf2c
Review-Url: https://codereview.chromium.org/2910983002
Cr-Original-Commit-Position: refs/heads/master@{#475457}
Committed: https://chromium.googlesource.com/chromium/src/+/67618346e22720b39a26ff20d7b94f9b10cd5f22
Review-Url: https://codereview.chromium.org/2910983002
Cr-Commit-Position: refs/heads/master@{#475511}
Committed: https://chromium.googlesource.com/chromium/src/+/8d654d3a72c612695257f5cc33ab1625488df780
Patch Set 1 #Patch Set 2 : fix test failures #Patch Set 3 : fix ios failures, add random change to browsing_data_utils.h to trigger all tests #
Messages
Total messages: 38 (23 generated)
dullweber@chromium.org changed reviewers: + maxwalker@chromium.org, msramek@chromium.org
Hi, I changed all browsing_data strings to be consistently upper case and replaced the cookie counter text as discussed with Max. Please take a look
RS LGTM, but I defer to Max. Regarding uppercasing: I remember dbeam@'s feedback that the "none" string looks like there was an error in computation (like when JS outputs "undefined" or "NaN"). So I think uppercasing is a positive change, but I guess there must have been a reason why we've been using lowercase?
LGTM! I think we had this right in the design doc (https://screenshot.googleplex.com/R2QkYQgkOjF.png), but then I didn't pay enough attention when we first added the data counters to the UI. Thanks, both!
The CQ bit was checked by dullweber@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1496066424436640, "parent_rev":
"f5d5e10ae336f7618c7afe8cce1aa17b9d123dc8", "commit_rev":
"818885262b08d3e4439c8ae538cec8090b5cdf2c"}
Message was sent while issue was closed.
Description was changed from ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 ========== to ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 Review-Url: https://codereview.chromium.org/2910983002 Cr-Commit-Position: refs/heads/master@{#475344} Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce...
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2911923002/ by guidou@chromium.org. The reason for reverting is: Breaks Linux Tests bot. See: https://uberchromegw.corp.google.com/i/chromium.linux/builders/Linux%20Tests/... Sample logs: Google Test trace: ../../components/browsing_data/core/browsing_data_utils_unittest.cc:120: Test params: 0 password(s), 0 is_synced ../../components/browsing_data/core/browsing_data_utils_unittest.cc:122: Failure Expected: output Which is: None To be equal to: base::.
Message was sent while issue was closed.
Description was changed from ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 Review-Url: https://codereview.chromium.org/2910983002 Cr-Commit-Position: refs/heads/master@{#475344} Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce... ========== to ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 Review-Url: https://codereview.chromium.org/2910983002 Cr-Commit-Position: refs/heads/master@{#475344} Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce... ==========
The CQ bit was checked by dullweber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by dullweber@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from msramek@chromium.org, maxwalker@chromium.org Link to the patchset: https://codereview.chromium.org/2910983002/#ps20001 (title: "fix test failures")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496130855742870,
"parent_rev": "315dddc6bbfe2ca11f6d3b127f02e5fc66d71aae", "commit_rev":
"67618346e22720b39a26ff20d7b94f9b10cd5f22"}
Message was sent while issue was closed.
Description was changed from ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 Review-Url: https://codereview.chromium.org/2910983002 Cr-Commit-Position: refs/heads/master@{#475344} Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce... ========== to ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 Review-Url: https://codereview.chromium.org/2910983002 Cr-Original-Commit-Position: refs/heads/master@{#475344} Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce... Review-Url: https://codereview.chromium.org/2910983002 Cr-Commit-Position: refs/heads/master@{#475457} Committed: https://chromium.googlesource.com/chromium/src/+/67618346e22720b39a26ff20d7b9... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/67618346e22720b39a26ff20d7b9...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2911003003/ by kjellander@chromium.org. The reason for reverting is: Breaks ios-simulator bots: https://build.chromium.org/p/chromium.mac/builders/ios-simulator/builds/17967 I'll file about about why analyze decided that ios_chrome_unittests could be skipped when running tryjobs when this was relanded..
Message was sent while issue was closed.
Description was changed from ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 Review-Url: https://codereview.chromium.org/2910983002 Cr-Original-Commit-Position: refs/heads/master@{#475344} Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce... Review-Url: https://codereview.chromium.org/2910983002 Cr-Commit-Position: refs/heads/master@{#475457} Committed: https://chromium.googlesource.com/chromium/src/+/67618346e22720b39a26ff20d7b9... ========== to ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 Review-Url: https://codereview.chromium.org/2910983002 Cr-Original-Commit-Position: refs/heads/master@{#475344} Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce... Review-Url: https://codereview.chromium.org/2910983002 Cr-Commit-Position: refs/heads/master@{#475457} Committed: https://chromium.googlesource.com/chromium/src/+/67618346e22720b39a26ff20d7b9... ==========
On 2017/05/30 11:33:29, kjellander_chromium wrote: > A revert of this CL (patchset #2 id:20001) has been created in > https://codereview.chromium.org/2911003003/ by mailto:kjellander@chromium.org. > > The reason for reverting is: Breaks ios-simulator bots: > https://build.chromium.org/p/chromium.mac/builders/ios-simulator/builds/17967 > > I'll file about about why analyze decided that ios_chrome_unittests could be > skipped when running tryjobs when this was relanded.. I filed https://bugs.chromium.org/p/chromium/issues/detail?id=727654 to track this. I guess .grdp files aren't understood by analyze.
The CQ bit was checked by dullweber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dullweber@chromium.org changed reviewers: + lpromero@chromium.org
lpromero@chromium.org: Please review changes in ios/chrome/browser/ui/settings/clear_browsing_data_collection_view_controller_unittest.mm
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The dryrun succeeded and ios-simulator didn't skip ios_chrome_unittests this time. components_unittests and unit_tests have been executed by linux_chromium_rel_ng and linux_android_rel_ng, so I will try another reland.
The CQ bit was checked by dullweber@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from msramek@chromium.org, maxwalker@chromium.org Link to the patchset: https://codereview.chromium.org/2910983002/#ps40001 (title: "fix ios failures, add random change to browsing_data_utils.h to trigger all tests")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496150820927340,
"parent_rev": "11d8154eb4cc10a04f18b0b7a254995765aff569", "commit_rev":
"8d654d3a72c612695257f5cc33ab1625488df780"}
Message was sent while issue was closed.
Description was changed from ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 Review-Url: https://codereview.chromium.org/2910983002 Cr-Original-Commit-Position: refs/heads/master@{#475344} Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce... Review-Url: https://codereview.chromium.org/2910983002 Cr-Commit-Position: refs/heads/master@{#475457} Committed: https://chromium.googlesource.com/chromium/src/+/67618346e22720b39a26ff20d7b9... ========== to ========== Improve and fix counter texts Change all CBD counter texts to upper case first letter. Replace cookie counter text for consistency with other counters. BUG=681523 Review-Url: https://codereview.chromium.org/2910983002 Cr-Original-Original-Commit-Position: refs/heads/master@{#475344} Committed: https://chromium.googlesource.com/chromium/src/+/818885262b08d3e4439c8ae538ce... Review-Url: https://codereview.chromium.org/2910983002 Cr-Original-Commit-Position: refs/heads/master@{#475457} Committed: https://chromium.googlesource.com/chromium/src/+/67618346e22720b39a26ff20d7b9... Review-Url: https://codereview.chromium.org/2910983002 Cr-Commit-Position: refs/heads/master@{#475511} Committed: https://chromium.googlesource.com/chromium/src/+/8d654d3a72c612695257f5cc33ab... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/8d654d3a72c612695257f5cc33ab... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
