You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "mdedetrich (via GitHub)" <gi...@apache.org> on 2023/08/01 08:06:56 UTC

[GitHub] [incubator-pekko-connectors-kafka] mdedetrich opened a new pull request, #122: Add commands to apply and check codestyle

mdedetrich opened a new pull request, #122:
URL: https://github.com/apache/incubator-pekko-connectors-kafka/pull/122

   Equivalent of https://github.com/apache/incubator-pekko-connectors/pull/207 but for this project


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko-connectors-kafka] mdedetrich commented on a diff in pull request #122: Add commands to apply and check codestyle

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #122:
URL: https://github.com/apache/incubator-pekko-connectors-kafka/pull/122#discussion_r1280978593


##########
CONTRIBUTING.md:
##########
@@ -96,6 +96,12 @@ Example:
     * Details 2
     * Details 3
 
+## Applying code style to the project
+
+The project uses [scalafmt](https://scalameta.org/scalafmt/) to ensure code quality which is automatically checked on
+every PR. If you would like to check for any potential code style problems locally you can run `sbt checkCodeStyle`
+and if you want to apply the code style then you can run `sbt applyCodeStyle`.

Review Comment:
   > Should we suggest `+checkCodeStyle` and `+applyCodeStyle`? Ensures that code that is Scala version specific gets checked and/or updated too.
   
   Both the scalafmt and sbt-java-formatter formatters don't do typechecking which means they don't involve scalac.
   
   In other words, they will produce the exact same result regardless of the scala version, so the `+` is not required



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko-connectors-kafka] mdedetrich commented on a diff in pull request #122: Add commands to apply and check codestyle

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #122:
URL: https://github.com/apache/incubator-pekko-connectors-kafka/pull/122#discussion_r1280978593


##########
CONTRIBUTING.md:
##########
@@ -96,6 +96,12 @@ Example:
     * Details 2
     * Details 3
 
+## Applying code style to the project
+
+The project uses [scalafmt](https://scalameta.org/scalafmt/) to ensure code quality which is automatically checked on
+every PR. If you would like to check for any potential code style problems locally you can run `sbt checkCodeStyle`
+and if you want to apply the code style then you can run `sbt applyCodeStyle`.

Review Comment:
   > Should we suggest `+checkCodeStyle` and `+applyCodeStyle`? Ensures that code that is Scala version specific gets checked and/or updated too.
   
   Both the scalafmt and sbt-java-formatter formatters don't do typechecking which means they don't involve scalac/javac (both formatters just parse scala/java source code and format based on that).
   
   In other words, they will produce the exact same result regardless of the scala version, so the `+` is not required



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko-connectors-kafka] mdedetrich commented on a diff in pull request #122: Add commands to apply and check codestyle

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #122:
URL: https://github.com/apache/incubator-pekko-connectors-kafka/pull/122#discussion_r1281010994


##########
CONTRIBUTING.md:
##########
@@ -96,6 +96,12 @@ Example:
     * Details 2
     * Details 3
 
+## Applying code style to the project
+
+The project uses [scalafmt](https://scalameta.org/scalafmt/) to ensure code quality which is automatically checked on
+every PR. If you would like to check for any potential code style problems locally you can run `sbt checkCodeStyle`
+and if you want to apply the code style then you can run `sbt applyCodeStyle`.

Review Comment:
   > the problem is source folders like `scala-2`- these can be skipped if you don't run the formatting for all scala versions
   
   This is orthogonal to what we are talking about. All the `+` command in sbt does is it iterates through `crossScalaVersions` while setting `scalaVersion` and then runs the command after `+`. 
   
   sbt-scalafmt doesn't do anything different wrt `scalaVersion`, they completely ignore it. In fact the implementation of sbt-scalafmt just uses sbt to retrieve the sources from various configs and then the formatting is handled externally from sbt (see https://github.com/scalameta/sbt-scalafmt/blob/main/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala#L534-L535). If this wasn't the case then the scalafmt-native wouldn't even work because it has no concept of sbt's scala version since scalafmt-native doesn't even have anything to do with sbt.
   
   The issue we have is more specific to sbt-header rather than running commands across all scala versions in general, so I will add the `+` there.



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko-connectors-kafka] pjfanning commented on a diff in pull request #122: Add commands to apply and check codestyle

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #122:
URL: https://github.com/apache/incubator-pekko-connectors-kafka/pull/122#discussion_r1280313370


##########
CONTRIBUTING.md:
##########
@@ -96,6 +96,12 @@ Example:
     * Details 2
     * Details 3
 
+## Applying code style to the project
+
+The project uses [scalafmt](https://scalameta.org/scalafmt/) to ensure code quality which is automatically checked on
+every PR. If you would like to check for any potential code style problems locally you can run `sbt checkCodeStyle`
+and if you want to apply the code style then you can run `sbt applyCodeStyle`.

Review Comment:
   Should we suggest `+checkCodeStyle` and `+applyCodeStyle`? Ensures that code that is Scala version specific gets checked and/or updated too.



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko-connectors-kafka] pjfanning commented on a diff in pull request #122: Add commands to apply and check codestyle

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #122:
URL: https://github.com/apache/incubator-pekko-connectors-kafka/pull/122#discussion_r1280988464


##########
CONTRIBUTING.md:
##########
@@ -96,6 +96,12 @@ Example:
     * Details 2
     * Details 3
 
+## Applying code style to the project
+
+The project uses [scalafmt](https://scalameta.org/scalafmt/) to ensure code quality which is automatically checked on
+every PR. If you would like to check for any potential code style problems locally you can run `sbt checkCodeStyle`
+and if you want to apply the code style then you can run `sbt applyCodeStyle`.

Review Comment:
   we had problems with sbt-header and not running it for all scala versions - as an example



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko-connectors-kafka] pjfanning commented on a diff in pull request #122: Add commands to apply and check codestyle

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #122:
URL: https://github.com/apache/incubator-pekko-connectors-kafka/pull/122#discussion_r1280987840


##########
CONTRIBUTING.md:
##########
@@ -96,6 +96,12 @@ Example:
     * Details 2
     * Details 3
 
+## Applying code style to the project
+
+The project uses [scalafmt](https://scalameta.org/scalafmt/) to ensure code quality which is automatically checked on
+every PR. If you would like to check for any potential code style problems locally you can run `sbt checkCodeStyle`
+and if you want to apply the code style then you can run `sbt applyCodeStyle`.

Review Comment:
   the problem is source folders like `scala-2`- these can be skipped if you don't run the formatting for all scala versions



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko-connectors-kafka] mdedetrich commented on a diff in pull request #122: Add commands to apply and check codestyle

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #122:
URL: https://github.com/apache/incubator-pekko-connectors-kafka/pull/122#discussion_r1281010994


##########
CONTRIBUTING.md:
##########
@@ -96,6 +96,12 @@ Example:
     * Details 2
     * Details 3
 
+## Applying code style to the project
+
+The project uses [scalafmt](https://scalameta.org/scalafmt/) to ensure code quality which is automatically checked on
+every PR. If you would like to check for any potential code style problems locally you can run `sbt checkCodeStyle`
+and if you want to apply the code style then you can run `sbt applyCodeStyle`.

Review Comment:
   > the problem is source folders like `scala-2`- these can be skipped if you don't run the formatting for all scala versions
   
   This is orthogonal to what we are talking about. All the `+` command in sbt does is it iterates through `crossScalaVersions` while setting `scalaVersion` and then runs the command after `+`. 
   
   sbt-scalafmt doesn't do anything different wrt `scalaVersion`, they completely ignore it. In fact the implementation of sbt-scalafmt just uses sbt to retrieve the sources from various configs and then the formatting is handled externally from sbt (see https://github.com/scalameta/sbt-scalafmt/blob/main/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala#L534-L535). If this wasn't the case then the scalafmt-native wouldn't even work because it has no concept of sbt's scala version since scalafmt-native doesn't even have anything to do with sbt.
   
   The issue we have is more specific to sbt-header because it seems to do something weird with configurations rather than running commands across all scala versions in general, so I will add the `+` there.



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko-connectors-kafka] mdedetrich merged pull request #122: Add commands to apply and check codestyle

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich merged PR #122:
URL: https://github.com/apache/incubator-pekko-connectors-kafka/pull/122


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org