|
|
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. |
Descriptionntp-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 #
Messages
Total messages: 32 (23 generated)
Description was changed from ========== 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). ========== to ========== 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 ==========
Description was changed from ========== 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 ========== to ========== 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 ==========
sfiera@chromium.org changed reviewers: + treib@chromium.org
PTAL
lgtm Somewhat related: We had some discussions about removing all the special "ForDebugging" accessors, and instead having every (say) provider output some generic debug string. That would probably go well with adding a "ToDictionary" method on ContentSuggestions.
On 2017/03/16 14:30:05, Marc Treib wrote: > lgtm > > Somewhat related: We had some discussions about removing all the special > "ForDebugging" accessors, and instead having every (say) provider output some > generic debug string. That would probably go well with adding a "ToDictionary" > method on ContentSuggestions. I like it.
The CQ bit was checked by sfiera@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: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by sfiera@chromium.org
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
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_arm6...)
The CQ bit was checked by sfiera@chromium.org
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
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_...)
The CQ bit was checked by sfiera@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: 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_arm6...)
The CQ bit was checked by sfiera@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: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by sfiera@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org Link to the patchset: https://codereview.chromium.org/2757663002/#ps20001 (title: "Make llx work")
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": 1490108148884960, "parent_rev": "1e19c9d4d1268cf7f9048ac14825fe0ac4785740", "commit_rev": "378b4e8a2660f9dfc74abdc2b04f3ffcbdf93671"}
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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/+/378b4e8a2660f9dfc74abdc2b04f... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/378b4e8a2660f9dfc74abdc2b04f... |