Thank you so much for detailed explanation and the history.
I understood and it seems *ProcedureDeclarationChecker* should not be enabled. However, it seems *RedundantIfChecker* okay because there are only two errors for this across the code base. I have seen some rules have been added time to time, which do not change super a lot through code base. For example, https://github.com/apache/spark/commit/b0f5497e9520575e5082fa8ce8be5569f43abe74 https://github.com/apache/spark/commit/d717ae1fd74d125a9df21350a70e7c2b2d2b4786 https://github.com/apache/spark/commit/947b9020b0d621bc97661a0a056297e6889936d3 Thanks! 2016-05-16 12:05 GMT+09:00 Nicholas Chammas <nicholas.cham...@gmail.com>: > Relevant discussion from some time ago: > https://issues.apache.org/jira/browse/SPARK-3849?focusedCommentId=14168961&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14168961 > > In short, if enabling a new style rule requires sweeping changes > throughout the code base, then it should not be enabled. > > We've talked in the past about developing some way of enforcing new style > rules only on changed lines in a PR, allowing the project's style to > gradually improve over time without a sudden, sweeping change that breaks > everybody's workflow. So far nobody's been able to put such a system > together, as far as I know. > > Nick > > On Sun, May 15, 2016 at 9:51 PM Hyukjin Kwon <gurwls...@gmail.com> wrote: > >> Hi all, >> >> Lately, I made a list of rules currently not applied on Spark from >> http://www.scalastyle.org/rules-dev.html and then I tried to test them. >> >> I found two rules that I think might be helpful but I am not too sure. >> Could I ask both can be added? >> >> >> *RedundantIfChecker *(See >> http://www.scalastyle.org/rules-dev.html#org_scalastyle_scalariform_RedundantIfChecker >> ) >> It seems there are two usage of this. This simply checks if (cond) true >> else false or if (cond) false else true,which can be just cond or !cond >> >> >> *ProcedureDeclarationChecker *(See >> http://www.scalastyle.org/rules-dev.html#org_scalastyle_scalariform_ProcedureDeclarationChecker >> ) >> >> >> It seems this simply checks if functions has the return type `= :Unit` >> explicitly. This one seems right because it is written in >> https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide#SparkCodeStyleGuide-ReturnTypes >> >> >> However, it seems the number of occurrence is super a lot. (It seems >> roughly more than 800 times). It seems this will cause a lot of conflicts. >> >> >> >> Here is a list of rules not mentioned in scalastyle-config.xml just in >> case someone wants to know. >> >> *IndentationChecker* >> >> <check enabled="true" class="org.scalastyle.file.IndentationChecker" >> level="warning"> >> >> <parameters> >> >> <parameter name="tabSize">2</parameter> >> >> <parameter name="methodParamIndentSize">2</parameter> >> >> </parameters> >> >> </check> >> >> >> *BlockImportChecker* >> >> <check enabled="true" >> class="org.scalastyle.scalariform.BlockImportChecker" level="warning"/> >> >> >> *DeprecatedJavaChecker* >> >> <check enabled="true" >> class="org.scalastyle.scalariform.DeprecatedJavaChecker" level="warning"/> >> >> >> *EmptyClassChecker* >> >> <check enabled="true" >> class="org.scalastyle.scalariform.EmptyClassChecker" level="warning"/> >> >> >> *ForBraceChecker* >> >> <check enabled="true" class="org.scalastyle.scalariform.ForBraceChecker" >> level="warning"/> >> >> >> *LowercasePatternMatchChecker* >> >> <check enabled="true" >> class="org.scalastyle.scalariform.LowercasePatternMatchChecker" >> level="warning"/> >> >> >> *MultipleStringLiteralsChecker* >> >> <check enabled="true" >> class="org.scalastyle.scalariform.MultipleStringLiteralsChecker" >> level="warning"> >> >> <parameters> >> >> <parameter name="allowed">1</parameter> >> >> <parameter name="ignoreRegex">^\"\"$</parameter> >> >> </parameters> >> >> </check> >> >> >> *PatternMatchAlignChecker* >> >> <check enabled="true" >> class="org.scalastyle.scalariform.PatternMatchAlignChecker" >> level="warning"/> >> >> >> *ProcedureDeclarationChecker* >> >> <check enabled="true" >> class="org.scalastyle.scalariform.ProcedureDeclarationChecker" >> level="warning"/> >> >> >> *RedundantIfChecker* >> >> <check enabled="true" >> class="org.scalastyle.scalariform.RedundantIfChecker" level="warning"/> >> >> >> *ScalaDocChecker* >> >> <check enabled="true" class="org.scalastyle.scalariform.ScalaDocChecker" >> level="warning"> >> >> <parameters> >> >> <parameter name="ignoreRegex">(.*Spec$)|(.*SpecIT$)</parameter> >> >> </parameters> >> >> </check> >> >> >> *TodoCommentChecker* >> >> <checker enabled="true" >> class="org.scalastyle.scalariform.TodoCommentChecker" level="warning"> >> >> <parameters> >> >> <parameter default="TODO|FIXME" type="string" name="words"/> >> >> </parameters> >> >> </checker> >> >> >> *VarFieldChecker* >> >> <check enabled="true" class="org.scalastyle.scalariform.VarFieldChecker" >> level="warning"/> >> >> >> *VarLocalChecker* >> >> <check enabled="true" class="org.scalastyle.scalariform.VarLocalChecker" >> level="warning"/> >> >> >> *WhileChecker* >> >> <check enabled="true" class="org.scalastyle.scalariform.WhileChecker" >> level="warning"/> >> >> >> *XmlLiteralChecker* >> >> <check enabled="true" >> class="org.scalastyle.scalariform.XmlLiteralChecker" level="warning"/> >> >> >> >> Thank you very much!! >> >
