t52053518 commented on issue #136:
URL:
https://github.com/apache/logging-log4j-kotlin/issues/136#issuecomment-3340966126
Thanks for taking this on!
---
What'd be the ETA for 2 major releases if you're going with the deprecated
approach?
That said, I don't think you can fully achieve this with `ReplaceWith`, and
I think this might create a new bug in the intermediary build, if this is a
valid use case:
1. Someone wants to use `logger` property, so they import the new
`....autoprop.logger`
2. But they also want to use something else from the old file, e.g.
`loggerOf()` so they import the old `....logger` as well
3. Compiler will fail on overload resolution ambiguity with the duplicate
`logger` property
---
The performance impact is real. It causes a method invocation every time.
Larger projects easily have more than 100 classes, further causing LRU cache
misses. Finally, this cache is synchronized (usual bad), and causes thread
pinning with coroutines (bad for a Kotlin-specific library since
IntelliJ/Kotlin advocates for them).
I'd argue that performance is much more important for such a core library
than convenience. Extension methods (if even needed, as you can see they're
like cancer invading everything in scope, e.g. `123.logger.info { ... }`) can
be easily added by the caller even prior to 1.3.0, meanwhile we can't take this
code out (apart from forking, which we're doing now).
Finally, you might even argue that this (simply updating to 1.3.0) is
already a mini-regression for folks who have Lint fail on unused variables.
As much as you'd like to avoid breaking, I wonder if it would be better to
do it in one go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
