Spring cleaning - #3148
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
❕ Flaky test reportNo test was executed to be analysed. 🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
| <dependency> | ||
| <groupId>${project.groupId}</groupId> | ||
| <artifactId>apm-quartz-1-plugin</artifactId> | ||
| <artifactId>apm-quartz-plugin-1</artifactId> |
There was a problem hiding this comment.
[for reviewer] using same naming convention as grpc
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <parent> | ||
| <artifactId>apm-quartz-job-plugin</artifactId> | ||
| <artifactId>apm-quartz</artifactId> |
There was a problem hiding this comment.
[for reviewer] using apm-quartz as module name for all the other sub-modules, the -plugin sufix is kept for the instrumentation plugins (with an extra per-version suffix to fit the naming conventions).
| <name>${project.groupId}:${project.artifactId}</name> | ||
|
|
||
| <properties> | ||
| <version.quartz>2.3.1</version.quartz> |
There was a problem hiding this comment.
[for reviewer] the common part was compiled with quartz 2.x and contained some code that was only used with 2.x plugin, however making sure the common part is compiled with 1.x only is safer.
| <dependency> | ||
| <groupId>org.springframework</groupId> | ||
| <artifactId>spring-context-support</artifactId> | ||
| <version>${version.spring}</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
|
|
||
| <dependency> | ||
| <groupId>org.quartz-scheduler</groupId> | ||
| <artifactId>quartz-jobs</artifactId> | ||
| <version>${version.quartz}</version> | ||
| <scope>test</scope> | ||
| </dependency> |
There was a problem hiding this comment.
[for reviewer] those were only required for quartz 2.x
| @@ -124,24 +107,6 @@ void testJobWithResult() throws SchedulerException { | |||
| assertThat(transaction.getResult()).isEqualTo("this is the result"); | |||
| } | |||
|
|
|||
| @Test | |||
| void testDirectoryScan() throws SchedulerException, IOException { | |||
There was a problem hiding this comment.
[for reviewer] this was only relevant for quartz 2.x (hence the Assumptions call at start of test method.
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-maven-plugin</artifactId> | ||
| <!-- explicit version required as bom will not cover plugins, only dependencies --> | ||
| <version>${version.spring-boot}</version> |
There was a problem hiding this comment.
[for reviewer] the global spring-boot version property was removed because only used here in test application, thus in test code.
| <version.jackson>2.14.2</version.jackson> | ||
| <version.junit>5.9.1</version.junit> | ||
| <version.junit.vintage>4.13.2</version.junit.vintage> | ||
| <version.logback>1.2.3</version.logback> |
There was a problem hiding this comment.
[for reviewer] this wasn't used anymore.
|
/test |
3 participants
What does this PR do?
version.logbackThe
spring.versionin the mainpom.xmlis used for two things:There are a few other version properties that are used in a similar way:
version.jetty-serverversion.okhttp(this one is also used outside test code, defining it in instrumentation plugin might be better than globally).version.h2Having a single property here makes updating and reviewing dependabot PRs easier, so it might be relevant to keep that.
Maybe it could be enough to add a small comment in the
pom.xmlto make it more obvious that those are not actual dependencies and only used for testing outside of production code.Checklist
spring.versionproperty and similar ones (see above)I have updated CHANGELOG.asciidocN/A