Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2279483002:
Allow category["suggestions"] to be absent. (Closed)
DescriptionAllow category["suggestions"] to be absent.
APIs do not return empty lists, apparently. If a repeated field is
empty, then it will be absent in the JSON. Allow this, but make sure
that the empty category is created first.
Committed: https://crrev.com/90f20b329a00110db356c893dd01b8184d1c38cd
Cr-Commit-Position: refs/heads/master@{#414402}
Patch Set 1 #
Total comments: 5
Patch Set 2 : Address comments. #
Messages
Total messages: 20 (11 generated)
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...
sfiera@chromium.org changed reviewers: + treib@chromium.org
Discovered in the course of the dismissal change, but figured it could be separate.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
tschumann@chromium.org changed reviewers: + tschumann@chromium.org
lgtm https://codereview.chromium.org/2279483002/diff/1/components/ntp_snippets/ntp... File components/ntp_snippets/ntp_snippets_fetcher.cc (right): https://codereview.chromium.org/2279483002/diff/1/components/ntp_snippets/ntp... components/ntp_snippets/ntp_snippets_fetcher.cc:548: if (!category_value->GetList("suggestions", &suggestions)) { let's document this case, e.g., // Categories are allowed to have empty suggestion lists (which result in an absent field in JSON).
lgtm https://codereview.chromium.org/2279483002/diff/1/components/ntp_snippets/ntp... File components/ntp_snippets/ntp_snippets_fetcher.cc (right): https://codereview.chromium.org/2279483002/diff/1/components/ntp_snippets/ntp... components/ntp_snippets/ntp_snippets_fetcher.cc:540: const base::ListValue* suggestions = nullptr; nit: Move this down to where it's used. https://codereview.chromium.org/2279483002/diff/1/components/ntp_snippets/ntp... components/ntp_snippets/ntp_snippets_fetcher.cc:548: if (!category_value->GetList("suggestions", &suggestions)) { On 2016/08/24 21:35:22, tschumann wrote: > let's document this case, e.g., > // Categories are allowed to have empty suggestion lists (which result in an > absent field in JSON). +1 for comment
https://codereview.chromium.org/2279483002/diff/1/components/ntp_snippets/ntp... File components/ntp_snippets/ntp_snippets_fetcher.cc (right): https://codereview.chromium.org/2279483002/diff/1/components/ntp_snippets/ntp... components/ntp_snippets/ntp_snippets_fetcher.cc:540: const base::ListValue* suggestions = nullptr; On 2016/08/25 09:14:02, Marc Treib wrote: > nit: Move this down to where it's used. Done. https://codereview.chromium.org/2279483002/diff/1/components/ntp_snippets/ntp... components/ntp_snippets/ntp_snippets_fetcher.cc:548: if (!category_value->GetList("suggestions", &suggestions)) { On 2016/08/25 09:14:02, Marc Treib wrote: > On 2016/08/24 21:35:22, tschumann wrote: > > let's document this case, e.g., > > // Categories are allowed to have empty suggestion lists (which result in an > > absent field in JSON). > > +1 for comment Done.
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, tschumann@chromium.org Link to the patchset: https://codereview.chromium.org/2279483002/#ps20001 (title: "Address comments.")
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: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
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...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Allow category["suggestions"] to be absent. APIs do not return empty lists, apparently. If a repeated field is empty, then it will be absent in the JSON. Allow this, but make sure that the empty category is created first. ========== to ========== Allow category["suggestions"] to be absent. APIs do not return empty lists, apparently. If a repeated field is empty, then it will be absent in the JSON. Allow this, but make sure that the empty category is created first. Committed: https://crrev.com/90f20b329a00110db356c893dd01b8184d1c38cd Cr-Commit-Position: refs/heads/master@{#414402} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/90f20b329a00110db356c893dd01b8184d1c38cd Cr-Commit-Position: refs/heads/master@{#414402} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
