Conversation
rnystrom
left a comment
There was a problem hiding this comment.
Could you provide a unit test demonstrating where it fails?
|
@rnystrom Done 😊 |
|
@rnystrom @jessesquires After adding the test, I find this bug isn't that trivial😆. Should I add a changelog for it? |
|
@zhubofei yes please! I need to do a deeper audit to make sure this doesn't break anything haha. |
|
@zhubofei Thanks for the fix, what I dont understand is that it seems like the current production code is not affected by this bug at all, do u have any theory why this is the case? |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@lorixx has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
There was a problem hiding this comment.
@lorixx has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: ## Changes in this pull request Should map be a map of `IndexPath`s instead of arrays of `IndexPath`s? Also, we might need some unit tests on this part. I don't think it's covered. ### Checklist - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Pull Request resolved: #1205 Reviewed By: Ziewvater Differential Revision: D15962803 Pulled By: lorixx fbshipit-source-id: f207b69ef0ebad08cd72b14ba53101e56d1604fd
Summary: ## Changes in this pull request Should map be a map of `IndexPath`s instead of arrays of `IndexPath`s? Also, we might need some unit tests on this part. I don't think it's covered. ### Checklist - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Pull Request resolved: #1205 Reviewed By: Ziewvater Differential Revision: D15962803 Pulled By: lorixx fbshipit-source-id: f207b69ef0ebad08cd72b14ba53101e56d1604fd
Changes in this pull request
Should map be a map of
IndexPaths instead of arrays ofIndexPaths? Also, we might need some unit tests on this part. I don't think it's covered.Checklist
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.