Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Collection conformance for *SectionModel types #135

Merged
merged 3 commits into from
Aug 28, 2018

Conversation

benasher44
Copy link
Contributor

@benasher44 benasher44 commented Aug 20, 2018

Changes in this pull request

This adds Collection conformance to *SectionModel types, which eases upgrading to Swift-based, non-Dwifft diffing (most diff Collections).

Checklist

  • All tests pass. Demo project builds and runs.
  • I added tests, an experiment, or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have reviewed the contributing guide

@benasher44 benasher44 added this to the 0.2.0 milestone Aug 20, 2018
@benasher44 benasher44 force-pushed the basher/add-collection-conformance branch from 370bd84 to ed3f40c Compare August 20, 2018 23:01
@benasher44 benasher44 changed the title Add collection conformance for *SectionModel types Add Collection conformance for *SectionModel types Aug 20, 2018
asmallteapot
asmallteapot previously approved these changes Aug 21, 2018
jessesquires
jessesquires previously approved these changes Aug 27, 2018
Copy link
Contributor

@jessesquires jessesquires left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice 😎

@benasher44 benasher44 dismissed stale reviews from jessesquires and asmallteapot via 25a55de August 27, 2018 23:00
jessesquires
jessesquires previously approved these changes Aug 27, 2018
return self.cellViewModels.startIndex
}

public var endIndex: Int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, each of these declarations should have /// :nodoc: to exempt them from generating empty docs

@@ -167,3 +167,22 @@ public struct CollectionSectionViewModel: DiffableViewModel {
self.diffingKey = diffingKey
}
}

/// Collection support for diffing
extension CollectionSectionViewModel: Collection {
Copy link
Contributor

@jessesquires jessesquires Aug 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe say "CollectionSectionViewModel conforms to Collection"

or leave as /// :nodoc:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is CollectionSectionViewModel conforms to Collection more informative than CollectionSectionViewModel: Collection + the current comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry just feel like that comment is a bit redundant. Just in case, I add backticks around Collection, which might make it read a bit better.

@benasher44 benasher44 force-pushed the basher/add-collection-conformance branch from e69beed to 404dbc3 Compare August 28, 2018 16:39
@benasher44 benasher44 merged commit 3c17140 into master Aug 28, 2018
@benasher44 benasher44 deleted the basher/add-collection-conformance branch August 28, 2018 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants