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

Issue 2885383002: ReadingListDataSource returns items instead of Entries (Closed)

Created:
3 years, 7 months ago by gambard
Modified:
3 years, 6 months ago
Reviewers:
jif
CC:
chromium-reviews, ios-reviews+chrome_chromium.org, ios-reviews_chromium.org, pkl (ping after 24h if needed), noyau+watch_chromium.org, marq+watch_chromium.org, stkhapugin
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

ReadingListDataSource returns items instead of Entries The ReadingListEntry are used in the model. The UI layer should not depend on the ReadingListEntry. This CL move the creation of the ReadingListItems from the view controller to the mediator. BUG=721758, 688392 Review-Url: https://codereview.chromium.org/2885383002 Cr-Commit-Position: refs/heads/master@{#477281} Committed: https://chromium.googlesource.com/chromium/src/+/b9e02c078fa310d0169787bb7a2187384ee4fc18

Patch Set 1 #

Patch Set 2 : Remove unused headers #

Total comments: 14

Patch Set 3 : Address comments #

Patch Set 4 : Styling #

Unified diffs Side-by-side diffs Delta from patch set Stats (+123 lines, -100 lines) Patch
M ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm View 1 2 8 chunks +39 lines, -95 lines 0 comments Download
M ios/chrome/browser/ui/reading_list/reading_list_data_source.h View 2 chunks +8 lines, -1 line 0 comments Download
M ios/chrome/browser/ui/reading_list/reading_list_mediator.mm View 1 2 3 4 chunks +76 lines, -4 lines 0 comments Download

Messages

Total messages: 13 (5 generated)
gambard
PTAL.
3 years, 7 months ago (2017-05-17 12:09:00 UTC) #2
gambard
ping :)
3 years, 6 months ago (2017-06-02 14:59:05 UTC) #3
gambard
jif@: PTAL.
3 years, 6 months ago (2017-06-06 12:13:29 UTC) #5
jif
https://codereview.chromium.org/2885383002/diff/20001/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm File ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm (right): https://codereview.chromium.org/2885383002/diff/20001/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm#newcode87 ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm:87: // Fills section |sectionIdentifier| with the items from |array| ...
3 years, 6 months ago (2017-06-06 13:12:27 UTC) #6
gambard
Thanks, PTAL. https://codereview.chromium.org/2885383002/diff/20001/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm File ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm (right): https://codereview.chromium.org/2885383002/diff/20001/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm#newcode87 ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm:87: // Fills section |sectionIdentifier| with the items ...
3 years, 6 months ago (2017-06-06 14:12:00 UTC) #7
jif
lgtm
3 years, 6 months ago (2017-06-06 14:17:16 UTC) #8
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/2885383002/60001
3 years, 6 months ago (2017-06-06 14:19:54 UTC) #10
commit-bot: I haz the power
3 years, 6 months ago (2017-06-06 14:29:30 UTC) #13
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b9e02c078fa310d0169787bb7a21...

Powered by Google App Engine
This is Rietveld 408576698