added a collection view sample to the sample project #65
Conversation
…nrecoverable error
|
|
||
| - (NSArray *)sortDescriptors { | ||
| return @[ | ||
| [NSSortDescriptor sortDescriptorWithKey:@"numberOfCats" ascending:NO], |
|
LGTM, the change to |
|
Do you want to bump the podspec or wait until more stuff gets added? |
|
I guess the assert/exception change is worth a 0.0.1 bump. I'll add that and change to 'VOK_CDSELECTOR'. |
| } | ||
| //FOR TESTING ONLY, NOT NECESSARY | ||
| [self.tableView reloadData]; | ||
| } |
There was a problem hiding this comment.
This certainly predates your changes, but it feels to me like this method should be - (BOOL)reloadDataError:(NSError **)error and then maybe have - (void)reloadData as a convenience wrapper calling reloadDataError:.
|
|
||
| @property (strong, readonly) NSFetchedResultsController *fetchedResultsController; | ||
| @property (nonatomic, strong, readonly) NSFetchedResultsController *fetchedResultsController; |
There was a problem hiding this comment.
Question for anyone reading:
fetchedResultsController, managedObjectClass, and tableView are declared as readonly
I didn't think instance variables, setters, and getters would be auto-synthesized. And yet, they are!
In the m file _fetchedResultsController exists and appears to work correctly. Same with the other readonly properties.
There was a problem hiding this comment.
- I don't see a question.
- Otherwise, how would you set a readonly property (that wasn't computed)?
- setter is synthesized too?
There was a problem hiding this comment.
I guess the question was "Why did it ever work?"
but I found "...if your property is readonly AND you implement your own getter method, then an instance variable is not synthesized." So this is working correctly. Nevermind!
|
Ready for another review if anyone's interested. I noticed 2.0.0 is not tagged as a release on GitHub. Could @designatednerd or @chillpop tag the current master HEAD as 2.0.0? |
|
Thanks @brockboland . I think someone also needs to |
|
Yeah yeah, gimme a minute here jeez |
|
hold off on that. I don't think we have enough changes to justify a "release" of the current HEAD. I'm fine with this PR being included in 2.0.0 |
|
(also, this is GTP) |
|
I have bad news, @chillpop |
| if (self) { | ||
| NSString *cellIdentifier = NSStringFromClass([VOKCollectionViewCell class]); | ||
| [collectionView registerNib:[UINib nibWithNibName:cellIdentifier bundle:nil] | ||
| forCellWithReuseIdentifier:cellIdentifier]; |
There was a problem hiding this comment.
Was that the bad news you had for @chillpop ?
There was a problem hiding this comment.
probably that 2.0.0 is already tagged
|
GTP here too. @vokal-isaac do you want to take another look before this is merged? |
|
@vokal-isaac last chance, I'm merging this today if I don't hear any objection. |
|
(:goat:,) |
|
ommmmmmm |
added a collection view sample to the sample project
|
2.0.1 has been tagged and pushed to |
Including bonus whitespace and nitpicks!
This could be helpful in fixing bugs like #64
If anyone's interested the datasources could definitely use some updating. I don't think they've been touched in a while.