eclipse / microprofile-metrics

microprofile-metrics
Apache License 2.0
100 stars 66 forks source link

Update and fix dependencies to use deps from parent pom/boms + update TCK README #753

Closed Channyboy closed 1 year ago

Channyboy commented 1 year ago

fixes #752

JanWesterkamp-iJUG commented 1 year ago

Adding a dependecy to the Jakarta EE 10 Core Profile solves the version deviation for CDI and REST. Doing it like this means on the other hand, that a MP 6.0 implementations needs to be compatible with the hole Core Profile, not only Jakarta REST, Jakarta CDI compatible implementations. It this is the intention, than this is fine. The sad story is, that for especially CDI additional dependencies are required and they are not in sync with the final Core Profile versions (but close only) - so in that way they need to be defined (and maintained) too (aligned with their Core Profile versions), because CDI transitive dependencies will not fit.

Regrading the version property naming (new use in README files), I prefer the Maven convention to use a postfix "version" in conjunction with a "." separator. I raised an issue for that (https://github.com/eclipse/microprofile-parent/issues/63), but it was ignored for the MP Parent 3.0 release unfortunaltely. Anyway I would suggest not to use it in a prefix notation, even when it's only documentation. Hopefully the next (major) release of the MP Parent can fix this. Using the dependencies from the parent (or TCK BOM) direcly, prevents us from redefining the versions somewhere else. The tck-bom should define the to Arquillian fitting test dependencies - for 1.7.0.Alpha12 it is well defined for the next version (3.1+ or 4.0+), but for the current 3.0 (with Arquillian 1.7.0.Alpha10) this need to be checked again.

Channyboy commented 1 year ago

@JanWesterkamp-iJUG Yes I believe @Emily-Jiang mentioned we should be pulling the core as a dependency.

Can change property to postfix the version.

Are you saying we need to explicitly define 1.7.0.Alpha12 for this PR?

JanWesterkamp-iJUG commented 1 year ago

@JanWesterkamp-iJUG Yes I believe @Emily-Jiang mentioned we should be pulling the core as a dependency.

Can change property to postfix the version.

Are you saying we need to explicitly define 1.7.0.Alpha12 for this PR?

@donbourne no. Roberto suggested to have one version in use for the hole MP and MP component specs, we are at version 3.0 of the parent and its tck-bom and the version defined there:

<properties>
    <!-- Dependencies - Test -->
    <version.testng>7.4.0</version.testng> <!-- aligns with Arquillian TestNG version -->
    <version.junit>4.13.1</version.junit>
    <version.hamcrest>1.3</version.hamcrest>
    <version.arquillian>1.7.0.Alpha10</version.arquillian>
</properties>

We updated them for the next release already, but unfortunately did not have done that before. So if the dependencies in the TCK declared without a specific version, the tck-bom ones will be used.

If you would like (or need) to deviate from this, we might consider to do a new MP Parent release with the recent updates. I hope (but did not checked yet) the old versions will work too and there is no issue like we have with log4j in the recent past left...

The current versions aligned with Arquillian are at the moment (and may be in that new parent release, master branch):

<properties>
    <!-- Dependencies - Test -->
    <!-- Aligns with Arquillian TestNG version -->
    <version.testng>7.5</version.testng>
    <!-- Aligns with Arquillian JUnit 4 version -->
    <version.junit>4.13.2</version.junit>
    <version.hamcrest>1.3</version.hamcrest>
    <version.arquillian>1.7.0.Alpha12</version.arquillian>
</properties>
Channyboy commented 1 year ago

@JanWesterkamp-iJUG updated to use tck-bom's 1.7.0.Alpha10