You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by Hyukjin Kwon <gu...@gmail.com> on 2016/05/16 01:50:55 UTC
Question about enabling some of missing rules.
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!!
Re: Question about enabling some of missing rules.
Posted by Hyukjin Kwon <gu...@gmail.com>.
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 <ni...@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 <gu...@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 <ni...@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 <gu...@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!!
>>>>
>>>
Re: Question about enabling some of missing rules.
Posted by Nicholas Chammas <ni...@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 <gu...@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 <ni...@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 <gu...@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!!
>>>
>>
Re: Question about enabling some of missing rules.
Posted by Hyukjin Kwon <gu...@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 <ni...@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 <gu...@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!!
>>
>
Re: Question about enabling some of missing rules.
Posted by Nicholas Chammas <ni...@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 <gu...@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!!
>