You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/08/02 20:36:28 UTC

[GitHub] [kafka] mdedetrich opened a new pull request, #12475: MINOR; Update scalafmt to latest version

mdedetrich opened a new pull request, #12475:
URL: https://github.com/apache/kafka/pull/12475

   Updates scalafmt to the latest version. This not only means migrating
   the checkstyle/.scalafmt.conf file but updating spotless as well. Due
   to improvements in the new version of scalafmt, it now also
   alphabetically sorts imports.
   
   The diff to the actual .scala source files is due to the alphabetical sorting of imports. I verified that this PR passes spotless/scalacheck by running ` ./gradlew :spotlessScalaCheck` locally.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r957629873


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.9

Review Comment:
   Hmmm... it's a little strange that we now have to specify the version in two places (`gradle/dependencies.gradle` and `checkstyle/.scalafmt.conf`), and it might lead to some headaches when we want to upgrade to a newer version in the future.
   
   If there's no sane way to define the Scalafmt version in a single place (preferably `gradle/dependencies.gradle`), could we leave a comment in the dependencies file noting that the version will also have to be bumped in the Scalafmt configuration file?
   
   FWIW, I checked out the history for Scalafmt to see if we could get some additional context there, but the [PR](https://github.com/scalameta/scalafmt/pull/2843) that added the requirement for the `version` property didn't shed a lot of light.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on PR #12475:
URL: https://github.com/apache/kafka/pull/12475#issuecomment-1203547014

   @divijvaidya This is one of the first PR's in adding the linting/formatting using scalafmt/scalafix for the Scala sources within Kafka if you want to have a look. @cadonna Do you want to have a look as well since its part of kafka scala streams?


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r936511846


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.8
+runner.dialect = scala213

Review Comment:
   The Scala 2.13 dialect is backwards compatible with 2.12. I have applied the dialect when updating from scalafmt 2 to scalafmt 3 with various popular OSS Scala projects and there isn't any issues (in any case the Scala code that is used im Kafka is quite conservative)
   
   > I have verified that the style guide we follow for this project is not prescriptive about formatting of the docs. Hence, your change should be ok.
   
   Do you think it makes sense to format the docs or should I leave the PR as os?



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r936511846


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.8
+runner.dialect = scala213

Review Comment:
   The Scala 2.13 dialect is backwards compatible with 2.12. I have applied the dialect when updating from scalafmt 2 to scalafmt 3 with various popular OSS Scala projects and there isn't any issues (in any case the Scala code that is used im Kafka is quite conservative)



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on PR #12475:
URL: https://github.com/apache/kafka/pull/12475#issuecomment-1230225740

   Tagging @C0urante for visibility


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r958078922


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.9

Review Comment:
   So I found the release notes where this change was added, i.e. https://github.com/scalameta/scalafmt/releases/tag/v3.1.0 but unfortunately like the PR its not very informative as to why the version now has to be explicit.
   
   In any case I have rebased the PR by adding a comment notifying that whenever `scalafmt` is bumped in `dependencies.gradle` that it also needs to be changed in `checkstyle/.scalafmt.conf` as requested.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r936511846


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.8
+runner.dialect = scala213

Review Comment:
   The Scala 2.13 dialect is backwards compatible with 2.12. I have applied the dialect when updating from scalafmt 2 to scalafmt 3 with various popular OSS Scala projects and there isn't any issues (in any case the Scala code that is used im Kafka is quite conservative)
   
   > I have verified that the style guide we follow for this project is not prescriptive about formatting of the docs. Hence, your change should be ok.
   
   Do you think it makes sense to format the docs or should I leave the PR as is?
   
   > Note that build is failing related to changes in this PR. We would probably need to fix the error but I think I might have already fixed it in MINOR: Catch InvocationTargetException explicitly and propagate underlying cause #12230 If that lands, then the check should pass here.
   
   In that case it's sensible to wait for your PR to land and then I can rebase.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r936006142


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.8
+docstrings.style = Asterisk
+docstrings.wrap = false

Review Comment:
   This keeps the current docstrings intact (i.e. forces scalafmt to ignore docstrings). Turning it on messes with manual html formatting that is used in docstrings



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r957629873


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.9

Review Comment:
   Hmmm... it's a little strange that we now have to specify the version in two places (`gradle/dependencies.gradle` and `checkstyle/.scalafmt.conf`), and it might lead to some headaches when we want to upgrade to a newer version in the future.
   
   If there's no sane way to define the Scalafmt version in a single place (preferably `gradle/dependencies.gradle`), could we leave a comment in the dependencies file noting that the version will also have to be bumped in the Scalafmt configuration file?



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] divijvaidya commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
divijvaidya commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r936828320


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.8
+runner.dialect = scala213

Review Comment:
   > Do you think it makes sense to format the docs or should I leave the PR as is?
   I think we should leave the PR as is. Converting the docs in the entire project will be cumbersome without any benefit. 
   
   > In that case it's sensible to wait for your PR to land and then I can rebase.
   Thanks, in that case, would you kindly review that PR please if you get some time? It's been pending for a while now waiting to get some attention.



##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.8
+runner.dialect = scala213

Review Comment:
   > Do you think it makes sense to format the docs or should I leave the PR as is?
   
   I think we should leave the PR as is. Converting the docs in the entire project will be cumbersome without any benefit. 
   
   > In that case it's sensible to wait for your PR to land and then I can rebase.
   
   Thanks, in that case, would you kindly review that PR please if you get some time? It's been pending for a while now waiting to get some attention.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] divijvaidya commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
divijvaidya commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r936426643


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.8
+runner.dialect = scala213

Review Comment:
   We still support scala 2.12 in this project.
   
   By specifying the dialect as 2.13, would we fail where syntax for 2.12 is used? If yes, are there any such cases in the project? (The build should tell us that after the existing spotless check passes).



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on PR #12475:
URL: https://github.com/apache/kafka/pull/12475#issuecomment-1224985118

   @ijuma So I have some good news, I managed to completely the solve the underlying problem upstream (see https://github.com/diffplug/spotless/issues/1273#issuecomment-1224978741). The PR has been updated/rebased and as you can see `spotlessScalaCheck` even works when Scala 2.12 is set, i.e.
   
   ```sh
   ./gradlew -PscalaVersion=2.12 :spotlessScalaCheck
   ```
   
   Also tagging @mimaison for visibility


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r958380812


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.9

Review Comment:
   Thanks, I think that should be fine.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r958078922


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.9

Review Comment:
   So I found the release notes where this change was added, i.e. https://github.com/scalameta/scalafmt/releases/tag/v3.1.0 but unfortunately its not very informative as to why the version now has to be explicit.
   
   In any case I have rebased the PR by adding a comment notifying that whenever `scalafmt` is bumped in `dependencies.gradle` that it also needs to be changed in `checkstyle/.scalafmt.conf`



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on PR #12475:
URL: https://github.com/apache/kafka/pull/12475#issuecomment-1211144083

   @ijuma Thanks for the pickup.
   
   I have investigated the root cause of the problem, basically when you specify the scala version with `-PscalaVersion=2.12` it also happens to override the Scala version that is used by scalafmt and the latest version of scalafmt (3.5.8) only runs with Scala 2.13.x. This is normally a non issue because scalafmt is designed to be used like a cli tool, i.e. its meant to be run independently of your project.
   
   I created an issue upstream at https://github.com/diffplug/spotless/issues/1273


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on PR #12475:
URL: https://github.com/apache/kafka/pull/12475#issuecomment-1231234530

   > I've kicked off another Jenkins run to see if we can get at least one green build with Scala 2.12.
   
   So Jenkins did in fact pass this PR as green build. I have just pushed to this PR so another build has been triggered but https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-12475/8/pipeline is the build that passed.


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on PR #12475:
URL: https://github.com/apache/kafka/pull/12475#issuecomment-1221538822

   @ijuma So I have added a commit which adds a workaround for the Scala runtime conflict issues. The CI is failing but it appears it's not using the updated `Jenkinsfile` in the commit so I suspect that after a merge the Apache CI should use the updated Jenkinsfile?
   
   @mimaison For visibility as well.


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] ijuma commented on pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
ijuma commented on PR #12475:
URL: https://github.com/apache/kafka/pull/12475#issuecomment-1210703856

   The scala 2.12 build failed https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-12475/


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
C0urante commented on PR #12475:
URL: https://github.com/apache/kafka/pull/12475#issuecomment-1231567747

   @vvcephei @cadonna would either of you have time to double-check this quick Scalafmt bump?


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r936005663


##########
build.gradle:
##########
@@ -30,7 +30,7 @@ buildscript {
 }
 
 plugins {
-  id 'com.diffplug.spotless' version '5.12.5'
+  id 'com.diffplug.spotless' version '6.9.0'

Review Comment:
   This version needed to be updated otherwise spotless would bring resolve the wrong Scala version for scalafmt.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r937028065


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.8
+runner.dialect = scala213

Review Comment:
   > Thanks, in that case, would you kindly review that PR please if you get some time? It's been pending for a while now waiting to get some attention.
   
   I just landed a review of the 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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r936009435


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.8
+runner.dialect = scala213
+docstrings.style = Asterisk
+docstrings.wrap = false

Review Comment:
   This forces scalafmt to ignore docstrings when formatting code. The new version of scalafmt works on docstrings however it doesn't support manual html formatting which is done in kafka scala streams.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on PR #12475:
URL: https://github.com/apache/kafka/pull/12475#issuecomment-1222258165

   Actually lets wait on merging this PR, it may be possible to fix the underlying problem in spotless (see https://github.com/diffplug/spotless/issues/1273#issuecomment-1222221116)


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] mdedetrich commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r957547580


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.9

Review Comment:
   So I checked and its actually a requirement of the newer versions of Scalafmt, if you don't have the version there it complains with
   
   ```
   Caused by: java.util.NoSuchElementException: version [expected 3.5.9 or 3.5.9]: missing
           at metaconfig.Configured.get(Configured.scala:15)
           at com.diffplug.spotless.glue.scalafmt.ScalafmtFormatterFunc.<init>(ScalafmtFormatterFunc.java:46)
           at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:67)
           ... 130 more
   ```
   
   Not sure what the reasoning behind having to include the include the scalafmt version in the configuration file but aside from some hacky workaround it needs to be done this away.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on a diff in pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12475:
URL: https://github.com/apache/kafka/pull/12475#discussion_r957440709


##########
checkstyle/.scalafmt.conf:
##########
@@ -12,7 +12,10 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-docstrings = JavaDoc
+version = 3.5.9

Review Comment:
   Why are we specifying this version here? Isn't it already declared in `dependencies.gradle`?



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante merged pull request #12475: MINOR; Update scalafmt to latest version

Posted by GitBox <gi...@apache.org>.
C0urante merged PR #12475:
URL: https://github.com/apache/kafka/pull/12475


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org