Pil0tXia commented on code in PR #4719:
URL: https://github.com/apache/eventmesh/pull/4719#discussion_r1557787481
##########
eventmesh-connectors/eventmesh-connector-rocketmq/build.gradle:
##########
@@ -16,25 +16,33 @@
*/
List rocketmq = [
+ "org.apache.rocketmq:rocketmq-acl:$rocketmq_version",
"org.apache.rocketmq:rocketmq-client:$rocketmq_version",
- "org.apache.rocketmq:rocketmq-broker:$rocketmq_version",
"org.apache.rocketmq:rocketmq-common:$rocketmq_version",
- "org.apache.rocketmq:rocketmq-store:$rocketmq_version",
- "org.apache.rocketmq:rocketmq-namesrv:$rocketmq_version",
- "org.apache.rocketmq:rocketmq-tools:$rocketmq_version",
- "org.apache.rocketmq:rocketmq-remoting:$rocketmq_version",
- "org.apache.rocketmq:rocketmq-logging:$rocketmq_version",
- "org.apache.rocketmq:rocketmq-srvutil:$rocketmq_version",
"org.apache.rocketmq:rocketmq-filter:$rocketmq_version",
- "org.apache.rocketmq:rocketmq-acl:$rocketmq_version",
+ "org.apache.rocketmq:rocketmq-logging:$rocketmq_version",
+ "org.apache.rocketmq:rocketmq-remoting:$rocketmq_version",
"org.apache.rocketmq:rocketmq-srvutil:$rocketmq_version",
-
+ "org.apache.rocketmq:rocketmq-store:$rocketmq_version",
+ "org.apache.rocketmq:rocketmq-srvutil:$rocketmq_version"
]
dependencies {
api project(":eventmesh-openconnect:eventmesh-openconnect-java")
implementation project(":eventmesh-common")
implementation rocketmq
+ implementation("org.apache.rocketmq:rocketmq-broker:$rocketmq_version") {
+ // Remove logging backend implementations
+ exclude group: 'ch.qos.logback', module: 'logback-classic'
+ }
Review Comment:
It seems there's no way to exclude dependencies from a list. The writing
splitting up dependencies in this way makes the List's existence a bit awkward.
How about the `configurations` block? I tried adding one and `logback`
didn't pass in.
```
configurations {
implementation.exclude group: 'ch.qos.logback', module: 'logback-classic'
}
```
##########
eventmesh-openconnect/eventmesh-openconnect-java/build.gradle:
##########
@@ -32,4 +29,7 @@ dependencies {
compileOnly 'org.projectlombok:lombok'
annotationProcessor 'org.projectlombok:lombok'
+
+ testRuntimeOnly "org.apache.logging.log4j:log4j-core"
+ testRuntimeOnly "org.apache.logging.log4j:log4j-slf4j2-impl"
Review Comment:
How about depending on `eventmesh-common` module? To achieve "the logging
backends are now only in `eventmesh-common`".
##########
build.gradle:
##########
@@ -139,7 +139,59 @@ allprojects {
}
}
-task tar(type: Tar) {
+tasks.register('dist') {
+ subprojects.forEach { subProject ->
+ dependsOn("${subProject.path}:jar")
+ }
Review Comment:
I tried using the basic `dependsOn` statement, but then the plugin jar
packages is incomplete.
```suggestion
dependsOn("jar")
```
The existing writting is more complicated than the old one, is this the
simplest writting yet?
##########
build.gradle:
##########
@@ -139,7 +139,59 @@ allprojects {
}
}
-task tar(type: Tar) {
+tasks.register('dist') {
+ subprojects.forEach { subProject ->
+ dependsOn("${subProject.path}:jar")
+ }
+ def includedProjects =
+ ["eventmesh-common",
+ "eventmesh-meta:eventmesh-meta-api",
+ "eventmesh-metrics-plugin:eventmesh-metrics-api",
+ "eventmesh-protocol-plugin:eventmesh-protocol-api",
+ "eventmesh-retry:eventmesh-retry-api",
+ "eventmesh-runtime",
+ "eventmesh-security-plugin:eventmesh-security-api",
+ "eventmesh-spi",
+ "eventmesh-starter",
+ "eventmesh-storage-plugin:eventmesh-storage-api",
+ "eventmesh-trace-plugin:eventmesh-trace-api",
+ "eventmesh-webhook:eventmesh-webhook-api",
+ "eventmesh-webhook:eventmesh-webhook-admin",
+ "eventmesh-webhook:eventmesh-webhook-receive"]
+ outputs.dirs('dist/apps', 'dist/bin', 'dist/conf', 'dist/lib',
'dist/licenses')
+ doLast {
+ includedProjects.each {
+ def project = findProject(it)
Review Comment:
Better name it `subProject` to indicate that all `from project...` equals to
`${projectDir}/...` and all `into 'dist/xxx'` equals to `${rootDir}/dist/xxx`.
---
Maybe `${projectDir}` and `${rootDir}` will be useful to indicate the
relative and absolute path, because I spent 5 mins to prove the change equals
to the old one. 😁 But the current writting do looks like more concise, so it
depends on you.
##########
build.gradle:
##########
@@ -139,7 +139,59 @@ allprojects {
}
}
-task tar(type: Tar) {
+tasks.register('dist') {
+ subprojects.forEach { subProject ->
+ dependsOn("${subProject.path}:jar")
+ }
+ def includedProjects =
+ ["eventmesh-common",
+ "eventmesh-meta:eventmesh-meta-api",
+ "eventmesh-metrics-plugin:eventmesh-metrics-api",
+ "eventmesh-protocol-plugin:eventmesh-protocol-api",
+ "eventmesh-retry:eventmesh-retry-api",
+ "eventmesh-runtime",
+ "eventmesh-security-plugin:eventmesh-security-api",
+ "eventmesh-spi",
+ "eventmesh-starter",
+ "eventmesh-storage-plugin:eventmesh-storage-api",
+ "eventmesh-trace-plugin:eventmesh-trace-api",
+ "eventmesh-webhook:eventmesh-webhook-api",
+ "eventmesh-webhook:eventmesh-webhook-admin",
+ "eventmesh-webhook:eventmesh-webhook-receive"]
+ outputs.dirs('dist/apps', 'dist/bin', 'dist/conf', 'dist/lib',
'dist/licenses')
Review Comment:
I tried to delete this statement and nothing changes, so maybe there's no
need to keep it~
##########
build.gradle:
##########
@@ -160,50 +212,48 @@ task zip(type: Zip) {
}
}
-task installPlugin() {
- if (!new File("${rootDir}/dist").exists()) {
- return
+tasks.register('installPlugin') {
+ // Compute dependency and outputs eagerly
+ dependsOn('dist')
Review Comment:
I think `installPlugin` doesn't need to depend on `dist` or `jar`, according
to https://github.com/apache/eventmesh/pull/4719#discussion_r1554661834.
Depending on `dist` will significantly lengthen the time of task execution.
So the original `exists()` check will be better.
##########
build.gradle:
##########
@@ -139,7 +139,59 @@ allprojects {
}
}
-task tar(type: Tar) {
+tasks.register('dist') {
+ subprojects.forEach { subProject ->
+ dependsOn("${subProject.path}:jar")
+ }
+ def includedProjects =
+ ["eventmesh-common",
+ "eventmesh-meta:eventmesh-meta-api",
+ "eventmesh-metrics-plugin:eventmesh-metrics-api",
+ "eventmesh-protocol-plugin:eventmesh-protocol-api",
+ "eventmesh-retry:eventmesh-retry-api",
+ "eventmesh-runtime",
+ "eventmesh-security-plugin:eventmesh-security-api",
+ "eventmesh-spi",
+ "eventmesh-starter",
+ "eventmesh-storage-plugin:eventmesh-storage-api",
+ "eventmesh-trace-plugin:eventmesh-trace-api",
+ "eventmesh-webhook:eventmesh-webhook-api",
+ "eventmesh-webhook:eventmesh-webhook-admin",
+ "eventmesh-webhook:eventmesh-webhook-receive"]
+ outputs.dirs('dist/apps', 'dist/bin', 'dist/conf', 'dist/lib',
'dist/licenses')
+ doLast {
+ includedProjects.each {
+ def project = findProject(it)
+ logger.lifecycle('Install module: module: {}', project.name)
+ copy {
+ from project.jar.archivePath
+ into 'dist/apps'
+ }
+ copy {
+ from project.file('bin')
+ into 'dist/bin'
+ }
+ copy {
+ from project.file('conf')
+ from project.sourceSets.main.resources.srcDirs
+ into 'dist/conf'
+ duplicatesStrategy = DuplicatesStrategy.EXCLUDE
+ exclude 'META-INF'
+ }
+ copy {
+ from project.configurations.runtimeClasspath
+ into 'dist/lib'
+ exclude 'eventmesh*'
+ }
Review Comment:
I don't know if it equals to the original writting:
```gradle
copy {
into("${projectDir}/dist/lib")
from project.configurations.runtimeClasspath
}
if (rootProject.contains(project.name)) {
copy {
into "${rootDir}/dist/lib"
from "${projectDir}/dist/lib"
exclude "eventmesh-*"
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
