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

Follow Apple's doc, add NSOperation only after all configuration done. #2232

Merged
merged 1 commit into from Feb 28, 2018

Conversation

dreampiggy
Copy link
Contributor

@dreampiggy dreampiggy commented Feb 27, 2018

New Pull Request Checklist

  • I have read and understood the CONTRIBUTING guide

  • I have read the Documentation

  • I have searched for a similar pull request in the project and found none

  • I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)

  • I have added the required tests to prove the fix/feature I am adding

  • I have updated the documentation (if necessary)

  • I have run the tests and they pass

  • I have run the lint and it passes (pod lib lint)

This merge request fixes / reffers to the following issues:

Pull Request Description

Just a little improvement to fix potential issue (Though works well now, but it's like a wrong usage). See Apple's doc this one and this one

Important: You should make all necessary configuration and modifications to an operation object before adding it to a queue, because once added, the operation may be run at any time, which may be too late for a change to have the intended effect.

Important: You should always configure dependencies before running your operations or adding them to an operation queue. Dependencies added afterward may not prevent a given operation object from running.

@codecov-io
Copy link

Codecov Report

Merging #2232 into master will increase coverage by 0.25%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2232      +/-   ##
=========================================
+ Coverage   78.24%   78.5%   +0.25%     
=========================================
  Files          36      36              
  Lines        3733    3735       +2     
  Branches      341     342       +1     
=========================================
+ Hits         2921    2932      +11     
+ Misses        789     780       -9     
  Partials       23      23
Impacted Files Coverage Δ
SDWebImage/SDWebImageDownloader.m 82.17% <100%> (+0.15%) ⬆️
SDWebImage/SDWebImageDownloaderOperation.m 92.18% <0%> (+2.93%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6318776...776ce2b. Read the comment docs.

@dreampiggy dreampiggy merged commit 48dd190 into SDWebImage:master Feb 28, 2018
@dreampiggy dreampiggy deleted the improvement_add_operation branch February 28, 2018 03:42
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

2 participants