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

Improved the documentation #820

Closed
wants to merge 7 commits into from
Closed

Conversation

weyert
Copy link
Contributor

@weyert weyert commented Jun 21, 2017

Improved the documentation so it that the Getting Started guide calls out that you need to set dataSource-property of the IGListAdapter as I totally misse that.

@facebook-github-bot
Copy link
Contributor

@weyert updated the pull request - view changes

@@ -272,6 +272,7 @@ - (void)performUpdatesAnimated:(BOOL)animated completion:(IGListUpdaterCompletio
id<IGListAdapterDataSource> dataSource = self.dataSource;
UICollectionView *collectionView = self.collectionView;
if (dataSource == nil || collectionView == nil) {
IGLKLog(@"Warning: Your call to performUpdatesAnimated-method is ignored as dataSource or collectionView haven't been set.");
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of "performUpdatesAnimated-method" we can use %s and pass __PRETTY_FUNCTION__ 😄

@@ -308,6 +309,7 @@ - (void)reloadDataWithCompletion:(nullable IGListUpdaterCompletion)completion {
id<IGListAdapterDataSource> dataSource = self.dataSource;
UICollectionView *collectionView = self.collectionView;
if (dataSource == nil || collectionView == nil) {
IGLKLog(@"Warning: Your call to reloadDataWithCompletion-method is ignored as dataSource or collectionView haven't been set.");
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above 😊

@facebook-github-bot
Copy link
Contributor

@weyert updated the pull request - view changes

@jessesquires
Copy link
Contributor

thanks @weyert ! just a few comments 😄

Copy link
Contributor

@rnystrom rnystrom left a comment

Choose a reason for hiding this comment

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

+1 @jessesquires's notes

@weyert
Copy link
Contributor Author

weyert commented Jun 24, 2017

Thank you, I will make the changes :)

Fixed the issue with the failed merge of `upstream`
@facebook-github-bot
Copy link
Contributor

@weyert updated the pull request - view changes

@weyert
Copy link
Contributor Author

weyert commented Jun 24, 2017

@jessesquires @rnystrom made the change as requested 👍

@facebook-github-bot
Copy link
Contributor

@weyert updated the pull request - view changes

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.

Looks good! 🥇

@facebook-github-bot
Copy link
Contributor

@rnystrom has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants