ctubbsii commented on PR #698: URL: https://github.com/apache/creadur-rat/pull/698#issuecomment-4939014646
@Claudenw wrote: > Most of these seem fine to me, however, I am a bit concerned about the maven build changes. We build a maven plugin so I think we need the maven plugin plugin bits. I didn't remove anything that wasn't already defined elsewhere. Those were defined in the `pluginManagement` section solely for version management. They were redundant with what was declared in the ASF parent POM that this project uses. So, they were serving no purpose. No plugin execution or usage was removed, only the redundant version management. > Have you tries to build the entire package from scratch using these mods? Do you have experience building a maven plugin? I built the entire package using `mvn verify` and all the GitHub Actions CI checks also performed the build. This project has several integration tests that make use of the plugin in a small test project to validate that it still works. Aside from that, I don't know what you mean about "build the entire package from scratch". Also yes, I have experience building maven plugins. I have created several, and help maintain some that are widely used (like [formatter-maven-plugin](https://code.revelc.net/formatter-maven-plugin/plugin-info.html)). > I like where this change is heading, I am just a bit nervous about some of the more obscure bits. Which bits? I'm happy to explain any bits you don't understand. > Not really requesting changes but want to hold merging until I am comfortable. Please let me know what it would take for you to get comfortable with the changes. -- 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]
