-
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
Move some versions to properties section in pom.xml #444
Conversation
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.
Thanks! I added some comments inline.
build-parent/pom.xml
Outdated
</dependency> | ||
<dependency> | ||
<groupId>org.jsoup</groupId> | ||
<artifactId>jsoup</artifactId> | ||
<version>1.10.2</version> | ||
<version>${version.jsoup}</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.
Jsoup and money-api can be removed. They are not used anymore.
Maybe we will include them again in the BOM but for now, let's remove them.
build-parent/pom.xml
Outdated
@@ -25,7 +25,11 @@ | |||
|
|||
<properties> | |||
<!-- TODO: unify version properties names - version.foo-name --> | |||
<version.money-api>1.0.3</version.money-api> | |||
<version.jsoup>1.10.2</version.jsoup> | |||
<version.junit>4.12</version.junit> |
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.
Maybe let's add JUnit at the bottom above shrinkwrap as it's a test dependency.
I know we're not consistent but only the jandex dependency has the other convention for now so let's not make everything inconsistent.
We'll fix that globally once we have decided if we go one way or another.
build-parent/pom.xml
Outdated
<version.org.jandex>2.1.0.Beta3</version.org.jandex> | ||
<version.slf4j-jboss-logging>1.1.0.Final</version.slf4j-jboss-logging> |
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.
let's add it with jboss-logging and log4j versions a bit below and call it slf4j-jboss-logging.version
for now.
I have removed jsoup and money-api dependencies and relocated versions as per review comment. |
Looks good now, merging, thanks! |
Move some dependency versions to properties section