You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apex.apache.org by ilganeli <gi...@git.apache.org> on 2015/11/04 21:21:48 UTC

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

GitHub user ilganeli opened a pull request:

    https://github.com/apache/incubator-apex-core/pull/156

    [APEX-230] Set line length to 120

    * Set line length to 120 in CheckStyle. 
    * Removed COMMA from NoWhitespaceBefore check since this was also throwing an error. This was broken, and then subsequently fixed in a recent patch of CheckStyle but does not appear to have yet been incorporated into the plugin. See: https://github.com/checkstyle/checkstyle/issues/2089
    
    * I did not update the actual CodeStyle settings to automatically enforce wrapping for longer lines because how we want to do this is a more complex question and potentially outside the scope of this issue. For now, CheckStyle will simply throw an error if the line is too long.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ilganeli/incubator-apex-core SetLineLength120

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-apex-core/pull/156.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #156
    
----
commit 8742d60cb82b2941378f2980055f29ddd713fcc3
Author: Ilya Ganelin <il...@capitalone.com>
Date:   2015-11-04T20:18:49Z

    Set line length to 120 in CheckStyle. Removed COMMA from NoWhitespaceBeforeCheck since this was broken in an earlier CheckStyle patch. This addresses APEX-230

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by 243826 <gi...@git.apache.org>.
Github user 243826 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#discussion_r44502182
  
    --- Diff: apex_checks.xml ---
    @@ -97,6 +97,10 @@
           <message key="ws.notPreceded" value="WhitespaceAround: ''{0}'' is not preceded with whitespace."/>
         </module>
     
    +    <module name="LineLength">
    +      <property name="max" value="120"/>
    --- End diff --
    
    what's the source of this magic number? why not 80, or 100 or 150?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by tweise <gi...@git.apache.org>.
Github user tweise commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-153907554
  
    Understood. We need to make further checkstyle changes in conjunction with APEX-207 then. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-154201614
  
    @ilganeli Please update number of allowed code violations, as otherwise build fails. In the commit comments please include APEX-248 and APEX-249.
    
    Did you try upgrading checkstyle to 6.12?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-153907123
  
    @tweise The only reason I submitted the additional changes is that otherwise the number of CheckStyle violation exceeded the allowed limit causing it to fail in travis. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-153915374
  
    If the Checkstyle rules was unable to load then we would not have been able to build APEX at all. This maybe a problem just with travis


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-154558436
  
    @vrozov Where do I configure the allowed number of violations? I did update checkstyle and confirmed that it works with the latest version. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-153862925
  
    @ilganeli Please squash commits related to CheckStyle into a single commit and add reference to JIRA for every commit. It may still be a single pull request, but in general it will be better to have 2 different PR, so discussion/review of code style changes in StreamingContainer.java or any other file does not mix with discussion on CheckStyle rules.
    
    For checkstyle rules I think we need to agree on increasing wrapping ident to 4 and enforcing annotation placement. It will be also good to combine this with upgrade to 6.12 checkstyle.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-155904227
  
    @ilganeli see following issues:
    The pom of all child modules with existing failures need to be corrected and is causing failure of the pull request build
    1. api has 103 existing errors than 102 
    2. common has 124 existing errors. Currently 84
    3. bufferserver has 165 existing errors. Currently 83.
    4. engine has 2942 existing errors. currently 2072.
    
    I think another change that should accompany this is:
     .idea/codeStyleSettings.xml should have  the continuation indentation as '4'



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-154637569
  
    Number of violations are configured in modules pom.xml and project pom has configuration for the checkstyle version to use:
    
    ```
          <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-checkstyle-plugin</artifactId>
            <configuration>
              <maxAllowedViolations>2072</maxAllowedViolations>
            </configuration>
          </plugin>
    ```
    and
    ```
            <plugin>
              <groupId>org.apache.maven.plugins</groupId>
              <artifactId>maven-checkstyle-plugin</artifactId>
              <version>2.17</version>
              <dependencies>
                <dependency>
                  <groupId>com.puppycrawl.tools</groupId>
                  <artifactId>checkstyle</artifactId>
                  <version>6.11.2</version>
                </dependency>
              </dependencies>
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-153920928
  
    @ilganeli Ilya, I suggest to drop commits related to StreamingContainer.java from this PR and add few more changes to checkstyle:
    1. Upgrade to 6.12. Verify that  NoWhitespaceBefore allows COMMA in that version.
    2. Enforce annotations on a separate line.
    3. Increase wrapping indentation to 4.
    
    This should cover all known issues with checkstyle, so we can move forward with fixing checkstyle violations in the existing code. This should be done in one or few batch updates as voted by the community.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-153912866
  
    @ilganeli @tweise 
    I intentionally introduced a space and ran checkstyle and I don't get the error which Ilya has posted. This check is necessary. 
    
    [ERROR] src/main/java/com/datatorrent/stram/engine/StreamingContainer.java:[302,68] (whitespace) NoWhitespaceBefore: ',' is preceded with whitespace.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-154077920
  
    @vrozov @tweise I've made the suggested updates and updated the comment for this PR accordingly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-153908056
  
    @tweise What would you recommend as the best path forward for this patch? Remove the additional changes and leave the changes only limited to the checkstyle updates?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by 243826 <gi...@git.apache.org>.
Github user 243826 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#discussion_r44502992
  
    --- Diff: apex_checks.xml ---
    @@ -97,6 +97,10 @@
           <message key="ws.notPreceded" value="WhitespaceAround: ''{0}'' is not preceded with whitespace."/>
         </module>
     
    +    <module name="LineLength">
    +      <property name="max" value="120"/>
    --- End diff --
    
    Thanks and sorry. I see that discussion now.
    
    On Tue, Nov 10, 2015 at 9:13 PM, Ilya Ganelin <no...@github.com>
    wrote:
    
    > In apex_checks.xml
    > <https://github.com/apache/incubator-apex-core/pull/156#discussion_r44502353>
    > :
    >
    > > @@ -97,6 +97,10 @@
    > >        <message key="ws.notPreceded" value="WhitespaceAround: ''{0}'' is not preceded with whitespace."/>
    > >      </module>
    > >
    > > +    <module name="LineLength">
    > > +      <property name="max" value="120"/>
    >
    > This was agreed in discussion on the mailing list. Thank you, Ilya Ganelin
    > -----Original Message----- From: Chetan Narsude [notifications@github.com
    > <ma...@github.com>] Sent: Wednesday, November 11, 2015
    > 12:10 AM Eastern Standard Time To: apache/incubator-apex-core Cc: Ganelin,
    > Ilya Subject: Re: [incubator-apex-core] [APEX-230] Set line length to 120 (
    > #156 <https://github.com/apache/incubator-apex-core/pull/156>) In
    > apex_checks.xml<#156 (comment)
    > <https://github.com/apache/incubator-apex-core/pull/156#discussion_r44502182>
    > >:
    > @@ -97,6 +97,10 @@ <message key="ws.notPreceded" value="WhitespaceAround:
    > ''{0}'' is not preceded with whitespace."/> </module> + <module
    > name="LineLength"> + <property name="max" value="120"/>
    > what's the source of this magic number? why not 80, or 100 or 150? — Reply
    > to this email directly or view it on GitHub<
    > https://github.com/apache/incubator-apex-core/pull/156/files#r44502182>.
    > ________________________________________________________ The information
    > contained in this e-mail is confidential and/or proprietary to Capital One
    > and/or its affiliates and may only be used solely in performance of work or
    > services for Capital One. The information transmitted herewith is intended
    > only for use by the individual or entity to which it is addressed. If the
    > reader of this message is not the intended recipient, you are hereby
    > notified that any review, retransmission, dissemination, distribution,
    > copying or other use of, or taking of any action in reliance upon this
    > information is strictly prohibited. If you have received this communication
    > in error, please contact the sender and delete the material from your
    > computer.
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/incubator-apex-core/pull/156/files#r44502353>.
    >



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#discussion_r44550623
  
    --- Diff: .idea/codeStyleSettings.xml ---
    @@ -102,5 +104,4 @@
         </option>
         <option name="USE_PER_PROJECT_SETTINGS" value="true" />
       </component>
    -</project>
    -
    +</project>
    --- End diff --
    
    The file is managed by IntelliJ and it does not end the file with EOL. I suggest we keep as is.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by "Chetan Narsude (cnarsude)" <cn...@cisco.com>.
What was this pull request/merge about? Looks spurious to me. Does it have
any impact besides introducing merge node in git?

‹
Chetan


On 11/11/15, 2:33 PM, "Chandni Singh" <ch...@datatorrent.com> wrote:

>Hi Illya,
>
>I didn't realize that the last commit in the pull request affected
>unrelated files and I merged it.
>
>Chandni
>
>On Wed, Nov 11, 2015 at 1:18 PM, asfgit <gi...@git.apache.org> wrote:
>
>> Github user asfgit closed the pull request at:
>>
>>     https://github.com/apache/incubator-apex-core/pull/156
>>
>>
>> ---
>> If your project is set up for it, you can reply to this email and have
>>your
>> reply appear on GitHub as well. If your project does not have this
>>feature
>> enabled and wishes so, or if the feature is enabled but not working,
>>please
>> contact infrastructure at infrastructure@apache.org or file a JIRA
>>ticket
>> with INFRA.
>> ---
>>


Re: [GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by Chandni Singh <ch...@datatorrent.com>.
Hi Illya,

I didn't realize that the last commit in the pull request affected
unrelated files and I merged it.

Chandni

On Wed, Nov 11, 2015 at 1:18 PM, asfgit <gi...@git.apache.org> wrote:

> Github user asfgit closed the pull request at:
>
>     https://github.com/apache/incubator-apex-core/pull/156
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-apex-core/pull/156


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by tweise <gi...@git.apache.org>.
Github user tweise commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-153904513
  
    Ilya, the intention behind https://malhar.atlassian.net/browse/APEX-207 is to address code style in a comprehensive manner. Let's not work on individual files and individual rules. I believe @chandnisingh is planning to take it up. Personally, I don't think we can expect to reformat entire files in one IDE and match exactly what another IDE would produce. We should instead agree on priority basis the basic set of rules that we want to enforce via checkstyle and then fix existing violations. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-155221511
  
    Here is the ticket for Anonymous class brace placement
    https://malhar.atlassian.net/browse/APEX-227
    
    and the ticket for upgrading checkstyle version
    https://malhar.atlassian.net/browse/APEX-239


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by 243826 <gi...@git.apache.org>.
Github user 243826 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#discussion_r44502174
  
    --- Diff: .idea/codeStyleSettings.xml ---
    @@ -102,5 +104,4 @@
         </option>
         <option name="USE_PER_PROJECT_SETTINGS" value="true" />
       </component>
    -</project>
    -
    +</project>
    --- End diff --
    
    end the files with a new line character.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-155908016
  
    Sigh - I don't understand what I've done, it looks like something got messed up in the last change when I merged in the latest devel-3. Any thoughts on how to fix the problem?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by tweise <gi...@git.apache.org>.
Github user tweise commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#discussion_r43956730
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/engine/StreamingContainer.java ---
    @@ -7,7 +7,7 @@
      * "License"); you may not use this file except in compliance
      * with the License.  You may obtain a copy of the License at
      *
    - *   http://www.apache.org/licenses/LICENSE-2.0
    --- End diff --
    
    Cannot modify license headers. License headers need to be consistent across all file types and comply with automated license check. This is not just an IDE concern.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-155180823
  
    After upgrade to 6.12 we can also enforce anonymous classes code style. Can you please verify that this rule is either on be default or turn it on.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-155896108
  
    @chandnisingh Fixed all issues except for APEX-239



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#discussion_r44502353
  
    --- Diff: apex_checks.xml ---
    @@ -97,6 +97,10 @@
           <message key="ws.notPreceded" value="WhitespaceAround: ''{0}'' is not preceded with whitespace."/>
         </module>
     
    +    <module name="LineLength">
    +      <property name="max" value="120"/>
    --- End diff --
    
    This was agreed in discussion on the mailing list.
    
    
    
    Thank you,
    Ilya Ganelin
    
    
    
    -----Original Message-----
    From: Chetan Narsude [notifications@github.com<ma...@github.com>]
    Sent: Wednesday, November 11, 2015 12:10 AM Eastern Standard Time
    To: apache/incubator-apex-core
    Cc: Ganelin, Ilya
    Subject: Re: [incubator-apex-core] [APEX-230] Set line length to 120 (#156)
    
    
    In apex_checks.xml<https://github.com/apache/incubator-apex-core/pull/156#discussion_r44502182>:
    
    
    > @@ -97,6 +97,10 @@
    >        <message key="ws.notPreceded" value="WhitespaceAround: ''{0}'' is not preceded with whitespace."/>
    >      </module>
    >
    > +    <module name="LineLength">
    > +      <property name="max" value="120"/>
    
    
    what's the source of this magic number? why not 80, or 100 or 150?
    
    —
    Reply to this email directly or view it on GitHub<https://github.com/apache/incubator-apex-core/pull/156/files#r44502182>.
    ________________________________________________________
    
    The information contained in this e-mail is confidential and/or proprietary to Capital One and/or its affiliates and may only be used solely in performance of work or services for Capital One. The information transmitted herewith is intended only for use by the individual or entity to which it is addressed. If the reader of this message is not the intended recipient, you are hereby notified that any review, retransmission, dissemination, distribution, copying or other use of, or taking of any action in reliance upon this information is strictly prohibited. If you have received this communication in error, please contact the sender and delete the material from your computer.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: [APEX-230] Set line length to 12...

Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/156#issuecomment-153865258
  
    @ilganeli NoWhitespaceBefore Comma issue has been fixed in 6.11. What error was it throwing?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---