-
Notifications
You must be signed in to change notification settings - Fork 123
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
Findbugs annotations should have optional or provided dependency #31
Comments
This looks related to google/guava#2824 and google/guava#2721 to me. So my personal gut reaction is to keep the JSR305 annotations as a |
It is similar. |
Also google/guava#2960 We can switch to the checkerframework nullness annotations, but IIRC there isn't a good alternative for |
I guess my question here is, is this causing anyone issues, or is it just a potential optimization to remove the dependency? I'd rather "do whatever Guava does" by default (since they will have thought about it more than I can) but I recognize that loggers might have different requirements. |
It's not causing any issues. It just causing additional work for users who don't want to deploy it as they have to exclude it or live with the footprint of the JAR in their deployment. |
I think jsr305 is a tiny jar in terms of bytes, but I see your point. I'll watch out to see if Guava do anything about this and follow suit if they do. I think if people are really worried about JAR size for deployment they would be using Proguard or similar to strip dead code, so it doesn't feel like it should be a big deal to me. I'm closing this since there's nothing I plan to do right now about it, but rest assured I've not forgotten. I've added myself to the Guava bug tracking the same thing. |
Currently both
flogger
andflogger-system-backend
have a default (compile
) dependency oncom.google.code.findbugs:jsr305
which means it is packaged with every application. It this not required as Findbugs annotations are not required to run flogger. A better solution would be to either declare the dependency asoptional
or change the scope toprovided
.The text was updated successfully, but these errors were encountered: