You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by "Pi Song (JIRA)" <ji...@apache.org> on 2008/02/29 12:48:52 UTC

[jira] Created: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Incorporate CheckStyle into Pig build.xml  (experimental)
---------------------------------------------------------

                 Key: PIG-128
                 URL: https://issues.apache.org/jira/browse/PIG-128
             Project: Pig
          Issue Type: Improvement
    Affects Versions: 0.1.0
            Reporter: Pi Song
             Fix For: 0.1.0


As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.

*Proposal*

+Stage1+
- Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt
- At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
- Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
- From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.

+Stage2+ (don't know when yet)
- It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)

>From my test, currently we have around 10000 violations. 

Awaiting for suggestions!!!


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Pi Song updated PIG-128:
------------------------

    Attachment: pig_checkstyle1.patch

This patch implements the proposal stage 1.

> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>         Attachments: pig_checkstyle1.patch
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Stefan Groschupf (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573951#action_12573951 ] 

Stefan Groschupf commented on PIG-128:
--------------------------------------

Great!
+1 for Stage 1.

> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>         Attachments: checkstyle-all-4.4.jar, PIG-128-v02.patch, pig_checkstyle1.patch
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Olga Natkovich (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Olga Natkovich updated PIG-128:
-------------------------------

    Patch Info: [Patch Available]

> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>         Attachments: checkstyle-all-4.4.jar, PIG-128-v02.patch, pig_checkstyle1.patch
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Pi Song updated PIG-128:
------------------------

    Description: 
As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.

*Proposal*

+Stage1+
- Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
- At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
- Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
- From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.

+Stage2+ (don't know when yet)
- It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)

>From my test, currently we have around 10000 violations. 

Awaiting for suggestions!!!


  was:
As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.

*Proposal*

+Stage1+
- Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt
- At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
- Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
- From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.

+Stage2+ (don't know when yet)
- It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)

>From my test, currently we have around 10000 violations. 

Awaiting for suggestions!!!



> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Benjamin Francisoud (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Benjamin Francisoud updated PIG-128:
------------------------------------

    Attachment: PIG-128-v02.patch

I don't know why but TortoiseMerge couldn't apply the pig_checkstyle1.patch, it was complaining about an unknown line ending at line 10 ?!

I applied the modification manually (luckily the patch wasn't big)

The patch is good in my opinion :)

Some remarks, I don't think we should create an other build-lib folder to hold checkstyle jar.
We should put it under lib for the moment. Eventually create sub folders in lib or (even better in my opinion) use ivy to manage such dependencies...

I changed the name of the checktyle config from pig_checks.xml to checktyle.xml (I think the name state more clearly what is in this file without opening it)

I improved build.xml:
* made the checkstyle warning display on screen or in a file configurable through the checkstyle.useFile property.
* replaced some tabs with whitespaces (but I'm becoming picky here ;) )
* added test / ... *.java to the list of files to verify

The checkstyle jar was missing, I will add it as an attachment in this bug.

> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>         Attachments: PIG-128-v02.patch, pig_checkstyle1.patch
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573774#action_12573774 ] 

Pi Song commented on PIG-128:
-----------------------------

- If you feel like we should also check test code, go for it.
- for build-lib, I want to seperate jars that we use to perform various things in the build process (Style Check, Code coverage, etc.. ) from jars that we need at runtime. If you have a better solution, also go for it!
- I generated the patch from eclipse, just found out that by default it doesn't include binary files. But then after I checked "include binary file" eclipse just gets frozen. Anyone knows a way to work around this?


People giving out suggestions. This is what I expected from creating this JIRA issue. However please don't miss out the more important question - *Do you agree with the approach?*

> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>         Attachments: checkstyle-all-4.4.jar, PIG-128-v02.patch, pig_checkstyle1.patch
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Benjamin Francisoud (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573776#action_12573776 ] 

Benjamin Francisoud commented on PIG-128:
-----------------------------------------

+1 for me 
I would even make the build fail if checkstyle find errors ;) 

> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>         Attachments: checkstyle-all-4.4.jar, PIG-128-v02.patch, pig_checkstyle1.patch
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574115#action_12574115 ] 

Pi Song commented on PIG-128:
-----------------------------

The modified patch looks alright to me. Though seems like someone has to apply patch and add the  jar file manually to commit.

> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>         Attachments: checkstyle-all-4.4.jar, PIG-128-v02.patch, pig_checkstyle1.patch
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Olga Natkovich (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573881#action_12573881 ] 

Olga Natkovich commented on PIG-128:
------------------------------------

+1 on doing this in experimental mode. I think we will have to wait and see how it works out before deciding whether it makes sense to enforce it.

> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>         Attachments: checkstyle-all-4.4.jar, PIG-128-v02.patch, pig_checkstyle1.patch
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (PIG-128) Incorporate CheckStyle into Pig build.xml (experimental)

Posted by "Benjamin Francisoud (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIG-128?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Benjamin Francisoud updated PIG-128:
------------------------------------

    Attachment: checkstyle-all-4.4.jar

checkstyle-all-4.4.jar needs to be put in the lib folder (at least in patch-v02)

> Incorporate CheckStyle into Pig build.xml  (experimental)
> ---------------------------------------------------------
>
>                 Key: PIG-128
>                 URL: https://issues.apache.org/jira/browse/PIG-128
>             Project: Pig
>          Issue Type: Improvement
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>             Fix For: 0.1.0
>
>         Attachments: checkstyle-all-4.4.jar, PIG-128-v02.patch, pig_checkstyle1.patch
>
>
> As discussed in the mailing list, now I have included CheckStyle as a part of the build process. Some might agree and some might not agree. Please note that initially *this is only for experimental purpose*. 
> In my opinion, this is a systematic way to control coding style as you expect more and more people coming to help, you will need a good system to support.
> *Proposal*
> +Stage1+
> - Checkstyle will run as a part of build process. The output file will be created at build/checkstyle/checkstyle-report.txt. This only took a few more seconds in my slow development box.
> - At the moment sun's guideline is used with special exceptions Indentation=4 and neglecting package.html requirement.
> - Failures on Checkstyle will not cause the build to be broken at this stage as this will only provide guideline for developers and for committers to make decisions whether the patch is ready to be committed. Basically new patches should not introduce more violations.
> - From time to time, we should spend some time cleaning up code to reduce the number of violations. Before, people just did clean-up and check-in believing the code would be cleaner. Now you will have a good indicator to showcase your achievement.
> +Stage2+ (don't know when yet)
> - It's interesting that some checks in Checkstyle can help us eliminate unforseen bugs such as DoubleCheckingLock, EqualsHashCode, MagicNumber, or StringLiteralEquality. These checks should be enforced as errors and break the build. The set of such hard checks needs us all to decide. (see http://checkstyle.sourceforge.net/config_coding.html)
> From my test, currently we have around 10000 violations. 
> Awaiting for suggestions!!!

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.