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

Logging decouple #110

Merged
merged 3 commits into from Dec 10, 2015

Conversation

Projects
None yet
2 participants
@timonus
Copy link
Contributor

timonus commented Dec 10, 2015

This decouples FLAnimatedImage from any specific logging libraries, logging is instead handled by passing a block that's invoked when logging should occur to FLAnimatedImage via +setLogBlock:. This resolves issue #107 which was preventing FLAnimatedImage from working in projects that also used Cocoa Lumberjack Logger when built with Carthage.

timonus added some commits Dec 3, 2015

Replace FLLog macros with utility class methods on FLAnimatedImage.
The logging functionality of FLAnimatedImage is now specified using a log block. This decouples FLAnimatedImage from CocoaLumberjackLogger and makes it so that logging is completely handled within a user-provided block.
@lukescholefield

This comment has been minimized.

Another way to do this would be add a none to the level:

typedef NS_ENUM(NSUInteger, FLLogLevel) {
    FLLogLevelNone = 0,
    FLLogLevelError,
    FLLogLevelWarn,
    FLLogLevelInfo,
    FLLogLevelDebug,
    FLLogLevelVerbose
};

(alternatively could be a bit mask).

After that, the log block could be + (void)setLogBlock:(void (^)(NSString *logString))logBlock; which would probably (for most users) end up just wrapping NSLog or similar.

We can also expose a log level variable that the parent application can set. Then, + (void)logString:(NSString *)logString withLevel:(FLLogLevel)level; can determine if we actually call the log block, so that the user doesn't have to handle the various cases within the block.

There are for and against reasons for this, but for most users it might be simpler.

@timonus

This comment has been minimized.

Copy link
Contributor

timonus commented Dec 10, 2015

Discussed a bit in person, going to leave as-is.

@lukescholefield

This comment has been minimized.

Copy link

lukescholefield commented Dec 10, 2015

👍

@timonus timonus merged commit c6b362b into master Dec 10, 2015

@timonus timonus deleted the logging-decouple branch Dec 10, 2015

timonus pushed a commit that referenced this pull request Dec 12, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment