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

Move assertions so they are valid. #1261

Merged
merged 1 commit into from
Dec 10, 2018

Conversation

wiseoldduck
Copy link
Member

If PIN_ANIMATED is enabled, the creation of a PINRemoteImageManager
will create a sharedDownloader also. Thus we cannot assert that it
doesn't exist. We will move this assertion up to the methods that
create the preconfiguredSharedManager and before said manager is
allocated.

If PIN_ANIMATED is enabled, the creation of a PINRemoteImageManager
will create a sharedDownloader also. Thus we cannot assert that it
doesn't exist. We will move this assertion up to the methods that
create the preconfiguredSharedManager and before said manager is
allocated.
@ghost
Copy link

ghost commented Dec 1, 2018

🚫 CI failed with log

@@ -131,7 +133,6 @@ + (void)setSharedImageManagerWithConfiguration:(nullable NSURLSessionConfigurati
+ (void)setSharedPreconfiguredRemoteImageManager:(PINRemoteImageManager *)preconfiguredPINRemoteImageManager
{
NSAssert(preconfiguredPINRemoteImageManager != nil, @"setSharedPreconfiguredRemoteImageManager requires a non-nil parameter");
NSAssert(sharedDownloader == nil, @"Singleton has been created and session can no longer be configured.");
Copy link
Member

Choose a reason for hiding this comment

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

But removing it here means I can get away with calling this method directly, even after a singleton was created? Having said that, I don't care that much tbh.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's true (if you managed to create a sharedDownloader but without a sharedPINRemoteImageManager, which will assert below) you could make your way through here and miss the asserts. It's definitely wrong as-is though.

@maicki
Copy link
Contributor

maicki commented Dec 10, 2018

@nguyenhuy Could you please restart the CI. Thanks!

@maicki maicki merged commit 5ae6547 into TextureGroup:master Dec 10, 2018
wsdwsd0829 pushed a commit to wsdwsd0829/Texture that referenced this pull request Mar 14, 2019
If PIN_ANIMATED is enabled, the creation of a PINRemoteImageManager
will create a sharedDownloader also. Thus we cannot assert that it
doesn't exist. We will move this assertion up to the methods that
create the preconfiguredSharedManager and before said manager is
allocated.
wsdwsd0829 pushed a commit to wsdwsd0829/Texture that referenced this pull request Mar 15, 2019
If PIN_ANIMATED is enabled, the creation of a PINRemoteImageManager
will create a sharedDownloader also. Thus we cannot assert that it
doesn't exist. We will move this assertion up to the methods that
create the preconfiguredSharedManager and before said manager is
allocated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants