-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Apply callback queue to downloader as well #240
Conversation
Current coverage is
|
其他应该没什么了。您想要一个项目的 commiter 权限么?这样以后讨论和改动也会方便一些。如果 OK 的话我稍后把你加成项目 maintainer。 |
Fix: |
I will merge it. |
Apply callback queue to downloader as well
ImageDownloader 里的 sessionDelegationQueue 是模仿 AFNetworking 里的处理,其实说起来整个改动的起因都是觉得 AFNetworking 里对 queue 的处理方式非常 elegant,从使用者的角度来说只需要配置好库选项,使用接口时完全不需要再考虑 queue dispatch,所以希望能把这一特性引入 KingfisherManager,目前看来应该是不成功,可能是因为 AF 里核心功能基本都在 manager 里,然而 Kingfisher 的核心逻辑是分在两个不同的类里 cache 和 downloader,所以不能直接照搬。接下来再进行尝试吧 另外对于这个 image extension 回调必定在 main queue 的保证 (与 options 中指定的 queue 无关。)我觉得如果从一个要提供给别人用的库角度讲,把这个逻辑写死在库里不一定是一个好选择。库提供的是不同的配置选项和用户选择了某个选项之后能够成功地实现该选项的效果,用户应该自己负责在使用库时选择正确的选项。在这个例子里就是 Kingfisher 提供默认和自定义两个选项,选择默认回调就会在主线程,选择自定义就会在自定义 queue。如果用户在 UIKit extension 这种应该在主线程回调的方法里却自定义了非 main queue 那么这是一个用户错误,应该改的是他自己的调用代码 最后我不是很清楚成为项目 maintainer 有什么其他权限,不过很荣幸可以成为 maintainer 的一员 |
Image Extension 因为是 UI 相关,回调是不应该有主线程以外的需求的,理念上来说涉及正误,而并不是灵活性的问题,我认为在库里直接帮助用户处理掉会更友好一些。现在我使用文档进行对此进行了一些说明,不过其实更好的做法是直接杜绝在 UIKit extension 方法中传入 queue 选项的可能性,添加 assert 也许是更好的选择。 maintainer 就是可以直接在这个项目里做 branch 和 commit,以及合并和关别人 issue 等等,大概就和自己的 repo 是一样的。 P.S. 可能的话还是尽量用英语交流,这样别的开发者也能参与进来。谢谢。 |
OK |
@dopcn
在你的基础上改动了一些:
ImageView
和UIButton
的 extension 中因为涉及的是 UI 操作,所以必须是 main thread;ImageCache
里从内存缓存中直接取的时候并不需要进行 dispatch,否则将造成刷新时的闪烁;因为从磁盘缓存读取为了性能考虑需要放在另外的线程执行,所以在内存无缓存但磁盘有缓存时刷新的闪烁似乎是不可避免的,不过因为这种情况并不是很常见或者致命。现在并不想添加像是在主线程读取磁盘缓存的选项,所以决定暂时先维持现状这样。稍后会在
optionsInfo
中添加一个强制进行 transition 的标识,如果用户觉得磁盘缓存读取的闪烁不可接受的话可以用像是 fade 之类的过度来缓解。如果没有什么其它问题的话可以应该 merge 了。