Failed to compile project when project uses CocoaLumberjack about flanimatedimage

[raphaelschaad]

First, awesome that you're using CocoaLumberjack, so are we, and I'm stoked about integrating FLAnimatedImage with it.

Second to address your build issue, I think I just adding 'const' to the declaration in your code should do the trick. I've just been following the CocoaLumberjack conventions: https://github.com/CocoaLumberjack/CocoaLumberjack/blob/master/Documentation/GettingStarted.md

[vinibaggio]

Interesting. We do change the log level in our codebase, but I don't know if it's ideal, and it's clearly not a FLAnimatedImage. Thanks for your response! This is a non-issue.

[raphaelschaad]

Ok, glad it works out! Please let me know how the logging (with the auto-detect logic of CocoaLumberjack in FLAnimatedImage) works for you!

Here's the relevant bit from the README:

It has fine-grained logging. By default, it uses NSLog. However, if your project uses CocoaLumberjack, it automatically can detect that and use CocoaLumberjack to send logs to the configured output.

And here's the relevant code in FLAnimatedImage (defaults to ON): https://github.com/Flipboard/FLAnimatedImage/blob/master/FLAnimatedImageDemo/FLAnimatedImage/FLAnimatedImage.h#L20

[vinibaggio]

@raphaelschaad what do you think about making this:

// Logging
// If set to 0, disables integration with CocoaLumberjack Logger (only matters if CocoaLumberjack is installed).
#define FLLumberjackIntegrationEnabled 1
// If set to 1, enables NSLog logging (only matters #if DEBUG -- never for release builds).
#define FLDebugLoggingEnabled 0

this:

// Logging
// If set to 0, disables integration with CocoaLumberjack Logger (only matters if CocoaLumberjack is installed).
#ifndef FLLumberjackIntegrationEnabled
  #define FLLumberjackIntegrationEnabled 1
#endif

// If set to 1, enables NSLog logging (only matters #if DEBUG -- never for release builds).

#define FLDebugLoggingEnabled 0

[raphaelschaad]

Would the intention be that you'd define FLLumberjackIntegrationEnabled in your own code and import that header in FLAnimatedImage.h?

[vinibaggio]

Yes! So we could disable that for now. Does that make sense?

[raphaelschaad]

That seems like a reasonable change. Just couple follow up questions to clarify:

• Is it to avoid modifying FLAnimatedImage.h? The easiest way to disable would be to just toggle the '1' to a '0' yourself.
• And if it's because you'd like to avoid modifying FLAnimatedImage.h, are you #importing the FLAnimatedImage header after you #define that in your code? How are you structuring your code so you don't need to import your header in FLAnimatedImage header (and hence change the file anyway)?
• And finally, did you run into any issues or is the FLAnimatedImage->Lumberjack logging too noisy, that you'd want to disable it?
  Thanks!

[vinibaggio]

@raphaelschaad
1- yes, we want to avoid changing files we don't own. Our Pods directory is actually ephemeral and not source controlled, so any changes we make to Pods are discarded.
2- We could define that before the import so we could toggle the feature off, or even add a GCC predefined macros via compilation flag (-D)
3- We rely on changing the loglevel on runtime, especially on testing, so our ddLogLevel is not constant, and we can't easily change that for now, sadly.

[raphaelschaad]

Ok, I think your proposed change makes sense in your case and doesn't harm. Would you want to open a PR? For consistency I think it would make sense to also wrap FLDebugLoggingEnabled into a #ifndef, would you agree?

Until your (local) change makes it into a new CocoaPods version, you can keep it -- when updating the next time it will be discarded and the new CocoaPods version will contain it (your PR).

Any ideas how we could integrate FLAnimatedImage<->CocoaLumberjack better into your logging stack? Is this even something you desire to have?

Thanks!

[vinibaggio]

I am happy to open a PR for that! I wanted to get your thoughts on the issue beforehand, though :)
And I agree with the consistency!

I don't think there's a better way to intergrate FLAnimatedImage<->CocoaLumberjack -- the problem is that we use it in a way that even CocoaLumberjack doesn't support, so I don't think there's a need for that to leak into FLAnimatedImage. I think the current approach is fine.

[raphaelschaad]

Ok, let's do this change then! Could you make sure the #define is indented with 4 spaces?

And can you live with the local change for now (I'd like to avoid cutting a new CocoaPod version just for this -- there's likely more improvements coming soon for a 1.0.3)?

[vinibaggio]

Yeah! I will submit as soon as possible. Thanks so much for your help and attention! :)

[raphaelschaad]

Great!