Re: [PR] MINOR: [S3599] Consider `NoDoubleBraceInitialization` Introduce `org.apache.kafka.openrewrite.SanityCheck` #21165 [kafka]
Pankraz76 commented on PR #21168: URL: https://github.com/apache/kafka/pull/21168#issuecomment-3681542240 it seems this pr is related as well: - https://github.com/apache/kafka/pull/21128 - https://docs.openrewrite.org/recipes/java/testing/junit5/cleanupassertions -- 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]
Re: [PR] MINOR: [S3599] Consider `NoDoubleBraceInitialization` Introduce `org.apache.kafka.openrewrite.SanityCheck` #21165 [kafka]
Pankraz76 commented on code in PR #21168:
URL: https://github.com/apache/kafka/pull/21168#discussion_r2628236715
##
build.gradle:
##
@@ -29,20 +29,22 @@ buildscript {
}
plugins {
- id 'com.github.ben-manes.versions' version '0.53.0'
id 'idea'
id 'jacoco'
id 'java-library'
+ id 'com.diffplug.spotless' version '8.1.0'
+ id 'com.github.ben-manes.versions' version '0.53.0'
+ id 'com.github.spotbugs' version '6.4.4' apply false
+ id 'com.gradleup.shadow' version '8.3.9' apply false
+ id 'io.swagger.core.v3.swagger-gradle-plugin' version "${swaggerVersion}"
+ id 'org.nosphere.apache.rat' version '0.8.1'
+ id 'org.openrewrite.rewrite' version '7.22.0' apply false
Review Comment:
correct.
This versatile SCA tool is already running in
[checkstyle](https://github.com/checkstyle/checkstyle/blob/e2c1b6a8cd1ea056a4ec275e319442ef6617014f/rewrite.yml#L20)
and [spotless](https://github.com/diffplug/spotless/pull/2756).
Highly welcome any feedback, thx.
--
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]
Re: [PR] MINOR: [S3599] Consider `NoDoubleBraceInitialization` Introduce `org.apache.kafka.openrewrite.SanityCheck` #21165 [kafka]
mjsax commented on code in PR #21168:
URL: https://github.com/apache/kafka/pull/21168#discussion_r2628239104
##
build.gradle:
##
@@ -29,20 +29,22 @@ buildscript {
}
plugins {
- id 'com.github.ben-manes.versions' version '0.53.0'
id 'idea'
id 'jacoco'
id 'java-library'
+ id 'com.diffplug.spotless' version '8.1.0'
+ id 'com.github.ben-manes.versions' version '0.53.0'
+ id 'com.github.spotbugs' version '6.4.4' apply false
+ id 'com.gradleup.shadow' version '8.3.9' apply false
+ id 'io.swagger.core.v3.swagger-gradle-plugin' version "${swaggerVersion}"
+ id 'org.nosphere.apache.rat' version '0.8.1'
+ id 'org.openrewrite.rewrite' version '7.22.0' apply false
id 'org.owasp.dependencycheck' version '12.1.8'
- id 'org.nosphere.apache.rat' version "0.8.1"
- id "io.swagger.core.v3.swagger-gradle-plugin" version "${swaggerVersion}"
Review Comment:
Thanks for the cleanup. Like it.
--
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]
Re: [PR] MINOR: [S3599] Consider `NoDoubleBraceInitialization` Introduce `org.apache.kafka.openrewrite.SanityCheck` #21165 [kafka]
mjsax commented on PR #21168: URL: https://github.com/apache/kafka/pull/21168#issuecomment-394069 @mumrah @chia7712 -- Any thought on this PR? -- 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]
Re: [PR] MINOR: [S3599] Consider `NoDoubleBraceInitialization` Introduce `org.apache.kafka.openrewrite.SanityCheck` #21165 [kafka]
Pankraz76 commented on code in PR #21168:
URL: https://github.com/apache/kafka/pull/21168#discussion_r2628236715
##
build.gradle:
##
@@ -29,20 +29,22 @@ buildscript {
}
plugins {
- id 'com.github.ben-manes.versions' version '0.53.0'
id 'idea'
id 'jacoco'
id 'java-library'
+ id 'com.diffplug.spotless' version '8.1.0'
+ id 'com.github.ben-manes.versions' version '0.53.0'
+ id 'com.github.spotbugs' version '6.4.4' apply false
+ id 'com.gradleup.shadow' version '8.3.9' apply false
+ id 'io.swagger.core.v3.swagger-gradle-plugin' version "${swaggerVersion}"
+ id 'org.nosphere.apache.rat' version '0.8.1'
+ id 'org.openrewrite.rewrite' version '7.22.0' apply false
Review Comment:
correct. This tool yet another SCA already running in
[checkstyle](https://github.com/checkstyle/checkstyle/blob/e2c1b6a8cd1ea056a4ec275e319442ef6617014f/rewrite.yml#L20)
and [spotless](https://github.com/diffplug/spotless/pull/2756).
Highly welcome any feedback, thx.
--
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]
Re: [PR] MINOR: [S3599] Consider `NoDoubleBraceInitialization` Introduce `org.apache.kafka.openrewrite.SanityCheck` #21165 [kafka]
mjsax commented on code in PR #21168:
URL: https://github.com/apache/kafka/pull/21168#discussion_r2628221443
##
build.gradle:
##
@@ -29,20 +29,22 @@ buildscript {
}
plugins {
- id 'com.github.ben-manes.versions' version '0.53.0'
id 'idea'
id 'jacoco'
id 'java-library'
+ id 'com.diffplug.spotless' version '8.1.0'
+ id 'com.github.ben-manes.versions' version '0.53.0'
+ id 'com.github.spotbugs' version '6.4.4' apply false
+ id 'com.gradleup.shadow' version '8.3.9' apply false
+ id 'io.swagger.core.v3.swagger-gradle-plugin' version "${swaggerVersion}"
+ id 'org.nosphere.apache.rat' version '0.8.1'
+ id 'org.openrewrite.rewrite' version '7.22.0' apply false
Review Comment:
If I read this correctly, `org.openrewrite.rewrite` is newly added? Others
are just reordered.
I am not familiar with this package. Curious to hear from others about it.
--
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]
Re: [PR] MINOR: [S3599] Consider `NoDoubleBraceInitialization` Introduce `org.apache.kafka.openrewrite.SanityCheck` #21165 [kafka]
Pankraz76 commented on code in PR #21168:
URL: https://github.com/apache/kafka/pull/21168#discussion_r2628219508
##
.github/workflows/build.yml:
##
@@ -33,23 +33,23 @@ on:
jobs:
configure:
runs-on: ubuntu-latest
-name: Configure Workflow
+name: Configure Workflow 🔧
outputs:
is-draft: ${{ steps.check-draft-pr.outputs.is-draft }}
test-catalog-days: ${{ steps.configure-outputs.outputs.days }}
sha: ${{ steps.configure-outputs.outputs.sha }}
steps:
- - name: Env
+ - name: Env 📊
run: printenv
env:
GITHUB_CONTEXT: ${{ toJson(github) }}
- - name: Check for Draft PR
+ - name: Check for Draft PR 📝
id: check-draft-pr
if: |
github.event_name == 'pull_request' &&
github.event.pull_request.draft
run: echo "is-draft=true" >> "$GITHUB_OUTPUT"
- - name: Configure Outputs
+ - name: Configure Outputs ⚙️
Review Comment:
thanks for the quick response. Revert.
--
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]
Re: [PR] MINOR: [S3599] Consider `NoDoubleBraceInitialization` Introduce `org.apache.kafka.openrewrite.SanityCheck` #21165 [kafka]
mjsax commented on code in PR #21168:
URL: https://github.com/apache/kafka/pull/21168#discussion_r2628208516
##
.github/workflows/build.yml:
##
@@ -33,23 +33,23 @@ on:
jobs:
configure:
runs-on: ubuntu-latest
-name: Configure Workflow
+name: Configure Workflow 🔧
outputs:
is-draft: ${{ steps.check-draft-pr.outputs.is-draft }}
test-catalog-days: ${{ steps.configure-outputs.outputs.days }}
sha: ${{ steps.configure-outputs.outputs.sha }}
steps:
- - name: Env
+ - name: Env 📊
run: printenv
env:
GITHUB_CONTEXT: ${{ toJson(github) }}
- - name: Check for Draft PR
+ - name: Check for Draft PR 📝
id: check-draft-pr
if: |
github.event_name == 'pull_request' &&
github.event.pull_request.draft
run: echo "is-draft=true" >> "$GITHUB_OUTPUT"
- - name: Configure Outputs
+ - name: Configure Outputs ⚙️
Review Comment:
Personally, I would not add this. :)
--
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]
Re: [PR] MINOR: [S3599] Consider `NoDoubleBraceInitialization` Introduce `org.apache.kafka.openrewrite.SanityCheck` #21165 [kafka]
Pankraz76 commented on code in PR #21168:
URL: https://github.com/apache/kafka/pull/21168#discussion_r2627928849
##
.github/workflows/build.yml:
##
@@ -33,23 +33,23 @@ on:
jobs:
configure:
runs-on: ubuntu-latest
-name: Configure Workflow
+name: Configure Workflow 🔧
outputs:
is-draft: ${{ steps.check-draft-pr.outputs.is-draft }}
test-catalog-days: ${{ steps.configure-outputs.outputs.days }}
sha: ${{ steps.configure-outputs.outputs.sha }}
steps:
- - name: Env
+ - name: Env 📊
run: printenv
env:
GITHUB_CONTEXT: ${{ toJson(github) }}
- - name: Check for Draft PR
+ - name: Check for Draft PR 📝
id: check-draft-pr
if: |
github.event_name == 'pull_request' &&
github.event.pull_request.draft
run: echo "is-draft=true" >> "$GITHUB_OUTPUT"
- - name: Configure Outputs
+ - name: Configure Outputs ⚙️
Review Comment:
just my 2ct. for styling this.
of course will undo on on slightest hint.
##
.github/workflows/build.yml:
##
@@ -111,37 +111,40 @@ jobs:
validate:
needs: [configure]
runs-on: ubuntu-latest
-name: Compile and Check (Merge Ref)
+name: Compile and Check (Merge Ref) 🛠️
steps:
- - name: Env
+ - name: Env 📊
run: printenv
env:
GITHUB_CONTEXT: ${{ toJson(github) }}
- - name: Checkout code
+ - name: Checkout code 📥
uses: actions/checkout@v5
with:
persist-credentials: false
- ref: ${{ github.sha }} # this is the default, just being explicit.
- - name: Setup Python
+ ref: ${{ github.sha }} # this is the default, just being explicit.
+ - name: Setup Python 🐍
uses: ./.github/actions/setup-python
- - name: Setup Gradle
+ - name: Setup Gradle 🐘
uses: ./.github/actions/setup-gradle
with:
java-version: 25
gradle-cache-read-only: ${{ !inputs.is-trunk }}
gradle-cache-write-only: ${{ inputs.is-trunk }}
develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
- - name: Compile and validate
+ - name: Compile and validate 🏛️
env:
SCAN_ARG: ${{ inputs.is-public-fork && '--no-scan' || '--scan' }}
# Gradle flags
# --build-cache: Let Gradle restore the build cache
# --info: For now, we'll generate lots of logs while setting
up the GH Actions
# --scan: Publish the build scan. This will only work on PRs
from apache/kafka and trunk
# --no-scan: For public fork PRs, we won't attempt to publish the
scan
-run: |
- ./gradlew --build-cache --info $SCAN_ARG check releaseTarGz -x test
- - name: Archive check reports
+run: ./gradlew --build-cache --info $SCAN_ARG check releaseTarGz -x
test
+ - name: Spotless ✨
+run: ./gradlew spotlessCheck --build-cache
Review Comment:
also wondering how spot is linked please, just as a question. Assuming its
already covered somehow with check phase before. Thx.
##
build.gradle:
##
@@ -29,20 +29,22 @@ buildscript {
}
plugins {
- id 'com.github.ben-manes.versions' version '0.53.0'
id 'idea'
id 'jacoco'
id 'java-library'
+ id 'com.diffplug.spotless' version '8.1.0'
+ id 'com.github.ben-manes.versions' version '0.53.0'
+ id 'com.github.spotbugs' version '6.4.4' apply false
+ id 'com.gradleup.shadow' version '8.3.9' apply false
+ id 'io.swagger.core.v3.swagger-gradle-plugin' version "${swaggerVersion}"
+ id 'org.nosphere.apache.rat' version '0.8.1'
+ id 'org.openrewrite.rewrite' version '7.22.0' apply false
id 'org.owasp.dependencycheck' version '12.1.8'
- id 'org.nosphere.apache.rat' version "0.8.1"
- id "io.swagger.core.v3.swagger-gradle-plugin" version "${swaggerVersion}"
Review Comment:
if desired will undo.
##
clients/src/main/java/org/apache/kafka/common/requests/ShareAcknowledgeResponse.java:
##
@@ -88,12 +88,6 @@ public static ShareAcknowledgeResponse parse(Readable
readable, short version) {
);
}
-private static boolean
matchingTopic(ShareAcknowledgeResponseData.ShareAcknowledgeTopicResponse
previousTopic, TopicIdPartition currentTopic) {
Review Comment:
this now being the only one unused.
Asking to include here to make, or dedicate/extract?
Sanity Check made to be generic and still individual.
-
https://github.com/diffplug/spotless/blob/67f7df71cb29833c2f94789dc31e9ecff18a71c5/rewrite.yml#L3
##
.github/workflows/build.yml:
##
@@ -111,37 +111,40 @@ jobs:
validate:
needs: [configure]
runs-on: ubuntu-latest
-name: Compile and Check (Merge Ref)
+name: Compile and Check (Merge Ref) 🛠️
steps:
- - name: Env
+ - name: Env 📊
run: printenv
env:
