Refine AspectJ plugin build phase config #3284
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
kriegaex
force-pushed
the
improve-aspectj-build-2
branch
from
ebf31dd
to
37bf519
Compare
Document the pros and cons in the POM with this comment: Attention: aspectj-maven-plugin MUST be declared AFTER maven-compiler-plugin, if they are both supposed to run in the same default phases 'compile' and 'test-compile', but execution order is to be guaranteed. Then, you have the convenience of being able to run e.g. 'mvn [clean] compile' instead of 'mvn [clean] process-classes'. For a slightly less convenient, but less refactoring-error-prone solution, see the commented-out phases below.
kriegaex
force-pushed
the
improve-aspectj-build-2
branch
from
37bf519
to
6e28a9f
Compare
|
@mp911de, like I said over there in the other PR, after your ANTLR optimisation and removal of Replacer plugin, I dropped the first commit because it is now obsolete, leaving only the AspectJ Maven phase change to default. I still think, this would help more than it would hurt. It is documented well enough to avoid anyone messing up in the future. And if they would mess up, they would notice immediately, because the build would break. |
|
Alright, we're on the same page now. Care to remove the commented-out XML bits so I can merge the PR without polishing it? Happy to keep the explanatory comment though. |
mp911de
changed the title
mp911de
added
type: task
|
Actually, I left them there on purpose, and the comment even points to them. I like things to be explicit rather than expecting other developers to possess sacred knowledge. But if you insist, of course I can remove them, as soon as I am near my workstation again. 🙂 |
|
OK, done, even though I still think that keeping the phases and the hint as a help for the future would be better. You can either merge the whole PR with both commits or cherry-pick just the first one, if you change your mind. |
mp911de
force-pushed
the
main
branch
2 times, most recently
from
52ee55f
to
61e6d36
Compare
This is a follow-up on #3282, where a reviewer's misunderstanding (running test builds with wrong Maven phases) led to parts of the PR getting falsely erased. This PR
not only reinstates those erased parts, but alsoenables developers who do not know the build configuration well enough to know that they should runmvn process-classesrather thanmvn compileto now use the latter. This new convenience feature is not a fix or change of the previous PR, but changes a build phase which has been there long before the PR.@mp911de, @odrotbohm