-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Override the Jandex version used by jandex-maven-plugin in build-parent #1921
Conversation
build-parent/pom.xml
Outdated
<dependency> | ||
<groupId>org.jboss</groupId> | ||
<artifactId>jandex</artifactId> | ||
<version>2.1.1.Final</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor but this is not properly indented: it should be 4 spaces. Could you fix it?
I think we need to fix having the version defined here but let's discuss that later and get this in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't it use ${jandex.version}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ah, I just asked about it in a previous review. We can't as it's defined in the bom and properties from the bom are not inherited.
I'm thinking about moving the version properties to the quarkus-parent artifact but that's an orthogonal discussion and we need this fixed sooner rather than later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah right.
Why is the plugin not releasing in alignment of Jandex? We should ask @n1hility : Would be nicer to have the plugin version share the version of jandex itself, so to guarantee synch (and express compatibility)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's a separate project for now but I could see how it would be nice to have it as one project.
We have a PR waiting here: wildfly/jandex-maven-plugin#13 (comment) .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gsmet Fixed :-)
3714dc8
to
aed0a9e
Compare
Unfortunately we cannot re-use the version from BOM.