Considering the references It seems *RedundantIfChecker *is okay then. Could I try to add this one?
*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 Thanks. 2016-05-16 12:56 GMT+09:00 Nicholas Chammas <nicholas.cham...@gmail.com>: > Ah I see, good references. Perhaps it's really then a committer judgement > call on how many changes become "too many" for a single PR. > 2016년 5월 15일 (일) 오후 11:16, Hyukjin Kwon <gurwls...@gmail.com>님이 작성: > >> 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!! >>>> >>>
