-
Notifications
You must be signed in to change notification settings - Fork 638
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
Initial support for @CustomLog #654
Conversation
Thank you very much for your work! I'm just returned from vacation and will look on your code next days. |
src/main/java/de/plushnikov/intellij/plugin/processor/clazz/log/AbstractLogProcessor.java
Outdated
Show resolved
Hide resolved
src/main/java/de/plushnikov/intellij/plugin/processor/clazz/log/AbstractLogProcessor.java
Show resolved
Hide resolved
src/main/java/de/plushnikov/intellij/plugin/processor/clazz/log/CustomLogProcessor.java
Show resolved
Hide resolved
src/main/java/de/plushnikov/intellij/plugin/processor/clazz/log/CustomLogProcessor.java
Outdated
Show resolved
Hide resolved
src/main/java/de/plushnikov/intellij/plugin/processor/clazz/log/CustomLogProcessor.java
Outdated
Show resolved
Hide resolved
src/main/java/de/plushnikov/intellij/plugin/processor/clazz/log/CustomLogProcessor.java
Outdated
Show resolved
Hide resolved
src/main/java/de/plushnikov/intellij/plugin/processor/clazz/log/CustomLogProcessor.java
Outdated
Show resolved
Hide resolved
src/main/java/de/plushnikov/intellij/plugin/processor/clazz/log/AbstractSimpleLogProcessor.java
Outdated
Show resolved
Hide resolved
For tests you can look on "lombok-intellij-plugin/testData/configsystem/..." |
@juriad are you planning to update your PR? |
@mplushnikov Yes, I intend to continue. I finally got some time allocated for this. |
@mplushnikov I pushed a new version which fixes all problems. I also had to adjust the grammar in |
Codecov Report
@@ Coverage Diff @@
## master #654 +/- ##
===========================================
- Coverage 69.35% 69.16% -0.2%
- Complexity 1712 1747 +35
===========================================
Files 196 199 +3
Lines 5936 6067 +131
Branches 1203 1247 +44
===========================================
+ Hits 4117 4196 +79
- Misses 1268 1289 +21
- Partials 551 582 +31
Continue to review full report at Codecov.
|
Thank you for your work! I just merged it in master! |
Thank you for your work. Do you know when new release will be available? 😉 |
Since support for @CustomLog (see projectlombok/lombok#2086) will be present in the next release of Lombok, I also added it into this Intellij plugin.
I have tested it briefly, however I have a couple of questions:
Performance.
The declaration is a quite complicated expression that requires parsing which can potentially be costly. How often are methods
validate
andgeneratePsiElements
called?If there would be a performance problem, we could introduce caching of parsed declaration, but again I don't know if we need to deal with multithreading or even what the cache key should be as the configuration value is found for a psiClass while lombok.config is only one per module.
Tests
I couldn't find a way how to unit test the implementation as we need to set lombok.config before the test is performed. Lombok itself allows to define a special comment which sets the configuration. https://github.com/rzwitserloot/lombok/pull/2086/files#diff-336a74e0014249db8ef0ef331af898f8R1 Do we have something similar here?
License
Shall I add myself to a list of contributors, or sign something in blood?
Is there anything that catches your eye, that I should change?