vpelikh commented on PR #4156:
URL: https://github.com/apache/logging-log4j2/pull/4156#issuecomment-5367055486

   Thanks for the review, @ramanathan1504! I've addressed both points.
   
   **Test refactor** — Good call on following the existing 
`PluginProcessorTest` pattern. I renamed the fixture to 
`FakePluginPublicSetter.java` and now read it straight out of 
`src/test/resources` via `getResource`, which eliminated the copy and the 
commons-io `FileUtils` usage. The generated-file output directory is now a 
JUnit `@TempDir`, so there's no manual delete (and no commons-io in this test) 
either.
   
   **`process()` simplification** — You're right that the annotation loop plus 
`contentEquals` was doing no work once `@SupportedAnnotationTypes` only 
declares `Plugin`. Reverted to the original `if (!annotations.isEmpty())` guard 
and appended the `processBuilderAttributeFields(roundEnv)` call, keeping it to 
two lines.
   
   One question: would you prefer I squash/rebase the review commits before 
this is merged, or leave the history as is? Happy to tidy things up whichever 
way you prefer.
   
   Let me know if there's anything else.


-- 
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]

Reply via email to