You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/07/14 10:41:35 UTC
[GitHub] [james-project] vttranlina opened a new pull request #538: Scala checkstyle
vttranlina opened a new pull request #538:
URL: https://github.com/apache/james-project/pull/538
In order to checkstyle for scala. Ref: https://github.com/linagora/james-project/issues/4348
- Tool: Scalafix (https://scalacenter.github.io/scalafix/)
- Maven plugin: scalafix-maven-plugin (https://github.com/evis/scalafix-maven-plugin)
**How to run:**
```bash
mvn -pl event-sourcing/event-sourcing-core,event-sourcing/event-sourcing-pojo,server/protocols/jmap-rfc-8621,server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common,mailbox/event/json,mdn clean compile test-compile scalafix:scalafix -Dscalafix.mode=AUTO_SUPPRESS_LINTER_ERRORS
```
Read more `scalafix.mode`: https://www.javadoc.io/doc/ch.epfl.scala/scalafix-interfaces/0.9.5/scalafix/interfaces/ScalafixMainMode.html
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #538: [SCALA_CHECKSTYLE] Setup scalafix & Implement some rules
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #538:
URL: https://github.com/apache/james-project/pull/538#issuecomment-882192460
Can you squash the fixups @vttranlina ?
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a change in pull request #538: Scala checkstyle
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #538:
URL: https://github.com/apache/james-project/pull/538#discussion_r670289171
##########
File path: .scalafix.conf
##########
@@ -1,9 +1,27 @@
rules = [
- RemoveUnused
+ RemoveUnused,
+ DisableSyntax,
+ ProcedureSyntax,
+ NoValInForComprehension,
+ LeakingImplicitClassVal
]
RemoveUnused.imports = true
RemoveUnused.privates = false
RemoveUnused.locals = true
RemoveUnused.patternvars = false
RemoveUnused.implicits = false
+
+DisableSyntax.noVars = false
+DisableSyntax.noThrows = false
+DisableSyntax.noNulls = false
+DisableSyntax.noReturns = false
+DisableSyntax.noWhileLoops = false
+DisableSyntax.noAsInstanceOf = false
+DisableSyntax.noIsInstanceOf = false
+DisableSyntax.noXml = false
+DisableSyntax.noDefaultArgs = false
+DisableSyntax.noFinalVal = true
+DisableSyntax.noFinalize = true
+DisableSyntax.noValPatterns = false
+DisableSyntax.noUniversalEquality = false
Review comment:
A bit surprised by your reasoning here. It's exactly because we spent a long time doing scala without a checkstyle that there might likely been a lot of wrong things in James.
If you do tell me though you want to solve them step by step in other PRs then I'm 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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #538: [SCALA_CHECKSTYLE] Setup scalafix & Implement some rules
Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #538:
URL: https://github.com/apache/james-project/pull/538
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #538: Scala checkstyle
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #538:
URL: https://github.com/apache/james-project/pull/538#issuecomment-879889503
Do we have it integrated by default to the build?
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a change in pull request #538: Scala checkstyle
Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #538:
URL: https://github.com/apache/james-project/pull/538#discussion_r670217501
##########
File path: .scalafix.conf
##########
@@ -1,9 +1,27 @@
rules = [
- RemoveUnused
+ RemoveUnused,
+ DisableSyntax,
+ ProcedureSyntax,
+ NoValInForComprehension,
+ LeakingImplicitClassVal
]
RemoveUnused.imports = true
RemoveUnused.privates = false
RemoveUnused.locals = true
RemoveUnused.patternvars = false
RemoveUnused.implicits = false
+
+DisableSyntax.noVars = false
+DisableSyntax.noThrows = false
+DisableSyntax.noNulls = false
+DisableSyntax.noReturns = false
+DisableSyntax.noWhileLoops = false
+DisableSyntax.noAsInstanceOf = false
+DisableSyntax.noIsInstanceOf = false
+DisableSyntax.noXml = false
+DisableSyntax.noDefaultArgs = false
+DisableSyntax.noFinalVal = true
+DisableSyntax.noFinalize = true
+DisableSyntax.noValPatterns = false
+DisableSyntax.noUniversalEquality = false
Review comment:
I tried to enable some config, but the result is not my expectations. It makes James code wrong.
So I think the first shoot is a simple config.
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a change in pull request #538: Scala checkstyle
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #538:
URL: https://github.com/apache/james-project/pull/538#discussion_r670297934
##########
File path: .scalafix.conf
##########
@@ -3,7 +3,8 @@ rules = [
DisableSyntax,
ProcedureSyntax,
NoValInForComprehension,
- LeakingImplicitClassVal
+ LeakingImplicitClassVal,
+ ExplicitResultTypes
Review comment:
Ah that's the ExplicitResultTypes then that need to match the scala version? Ok let's go without it first then
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #538: Scala checkstyle
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #538:
URL: https://github.com/apache/james-project/pull/538#discussion_r670290906
##########
File path: .scalafix.conf
##########
@@ -3,7 +3,8 @@ rules = [
DisableSyntax,
ProcedureSyntax,
NoValInForComprehension,
- LeakingImplicitClassVal
+ LeakingImplicitClassVal,
+ ExplicitResultTypes
Review comment:
Can we open 2 PRs?
-> One without ExplicitResultTypes that we can merge pretty much straight away
-> And a second one that eventually adds ExplicitResultTypes rule but depends on your external contribution to be merged and released?
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on pull request #538: Scala checkstyle
Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #538:
URL: https://github.com/apache/james-project/pull/538#issuecomment-880379065
> Do we have it integrated by default to the build?
With little to no changes on the code, and seeing the build green, I'm hardly convinced tbh
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a change in pull request #538: Scala checkstyle
Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #538:
URL: https://github.com/apache/james-project/pull/538#discussion_r670261654
##########
File path: .scalafix.conf
##########
@@ -3,7 +3,8 @@ rules = [
DisableSyntax,
ProcedureSyntax,
NoValInForComprehension,
- LeakingImplicitClassVal
+ LeakingImplicitClassVal,
+ ExplicitResultTypes
Review comment:
`ExplicitResultTypes` rule is the ci-error reason.
IMO, it is a useful rule. But we need an upgrade version from `scalafix-maven-plugin`.
I have a new pull request for it: https://github.com/evis/scalafix-maven-plugin/pull/15 (waiting approval from the maintainer)
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a change in pull request #538: Scala checkstyle
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #538:
URL: https://github.com/apache/james-project/pull/538#discussion_r670214045
##########
File path: .scalafix.conf
##########
@@ -1,9 +1,27 @@
rules = [
- RemoveUnused
+ RemoveUnused,
+ DisableSyntax,
+ ProcedureSyntax,
+ NoValInForComprehension,
+ LeakingImplicitClassVal
]
RemoveUnused.imports = true
RemoveUnused.privates = false
RemoveUnused.locals = true
RemoveUnused.patternvars = false
RemoveUnused.implicits = false
+
+DisableSyntax.noVars = false
+DisableSyntax.noThrows = false
+DisableSyntax.noNulls = false
+DisableSyntax.noReturns = false
+DisableSyntax.noWhileLoops = false
+DisableSyntax.noAsInstanceOf = false
+DisableSyntax.noIsInstanceOf = false
+DisableSyntax.noXml = false
+DisableSyntax.noDefaultArgs = false
+DisableSyntax.noFinalVal = true
+DisableSyntax.noFinalize = true
+DisableSyntax.noValPatterns = false
+DisableSyntax.noUniversalEquality = false
Review comment:
You seem to disable a lot of syntaxes here... Why? Is there some syntaxes we might wanna check?
--
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@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org