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

Issue 2757663002: ntp-snippets-internals: add fields, indent JSON (Closed)

Created:
3 years, 9 months ago by sfiera
Modified:
3 years, 9 months ago
Reviewers:
Marc Treib
CC:
chromium-reviews, noyau+watch_chromium.org, arv+watch_chromium.org, ntp-dev+reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

ntp-snippets-internals: add fields, indent JSON I thought I had gotten a notification about an article the server hadn't said to notify for. It actually had, but the dump didn't include all of the fields for content suggestions there. This adds the current set of fields, and indents the JSON so it's easier to scan for relevant information. Even though it's only used for the webui, I'd be inclined in the future to move PrepareSuggestion() into the component and give it a test (and a better name). CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2757663002 Cr-Commit-Position: refs/heads/master@{#458427} Committed: https://chromium.googlesource.com/chromium/src/+/378b4e8a2660f9dfc74abdc2b04f3ffcbdf93671

Patch Set 1 #

Patch Set 2 : Make llx work #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -2 lines) Patch
M chrome/browser/resources/snippets_internals.js View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/snippets_internals_message_handler.cc View 1 2 chunks +38 lines, -0 lines 0 comments Download

Messages

Total messages: 32 (23 generated)
sfiera
PTAL
3 years, 9 months ago (2017-03-16 14:24:47 UTC) #4
Marc Treib
lgtm Somewhat related: We had some discussions about removing all the special "ForDebugging" accessors, and ...
3 years, 9 months ago (2017-03-16 14:30:05 UTC) #5
sfiera
On 2017/03/16 14:30:05, Marc Treib wrote: > lgtm > > Somewhat related: We had some ...
3 years, 9 months ago (2017-03-16 14:44:09 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2757663002/1
3 years, 9 months ago (2017-03-16 15:40:22 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm64_dbg_recipe/builds/229173)
3 years, 9 months ago (2017-03-16 16:24:44 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2757663002/1
3 years, 9 months ago (2017-03-16 17:31:58 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_swarming_rel/builds/138237)
3 years, 9 months ago (2017-03-16 18:28:51 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2757663002/20001
3 years, 9 months ago (2017-03-21 14:56:01 UTC) #29
commit-bot: I haz the power
3 years, 9 months ago (2017-03-21 15:53:03 UTC) #32
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/378b4e8a2660f9dfc74abdc2b04f...

Powered by Google App Engine
This is Rietveld 408576698