Conversation
rfscholte
left a comment
rfscholte
left a comment
There was a problem hiding this comment.
Based on just the code this looks good and based on the comment this should be safe to do. I don't know where to expect any impact, if any.
asfgit
force-pushed
the
MNG-7020
branch
2 times, most recently
from
bd28681 to
06cfd2f
Compare
|
@rfscholte Since you approved this, can I go on and merge? |
|
IIRC this triggered a new issue. I'll remove my approval and need to have another look at it. |
|
Sure... |
There was a problem hiding this comment.
Unsure what to say here...
This was in place for Maven2 plugins (running in Maven3). Hopefully today they will break with Maven4, so WagonExcluder is really not needed as plugin will not even execute. Still, IMO we'd need some safety "thing" maybe simply reject plugins declaring dependency on Maven artifact(s) having version 2.x? Just to be on safe side? (like prerequisite, but other way around)
|
@cstamas @michael-o I thought we also worked on that "require maven-plugin-api version", but can't find the ticket. For me that must be implemented first before accepting this PR. |
That is true and that is why I have not yet merged it. |
|
@rfscholte Can you reassess please? |
mthmulders
left a comment
mthmulders
left a comment
There was a problem hiding this comment.
All integration tests are failing - could it be it's an outdated version of the IT's? Other than that, it looks fine to me.
|
Let me check... |
|
@mthmulders I have rebased the branch, I guess the ITs should pass. Let's wait. |
|
Resolve #8019 |
7 participants
No description provided.