You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apex.apache.org by chandnisingh <gi...@git.apache.org> on 2015/10/21 00:33:59 UTC
[GitHub] incubator-apex-core pull request: APEX-204 #resolve #comment setti...
GitHub user chandnisingh opened a pull request:
https://github.com/apache/incubator-apex-core/pull/125
APEX-204 #resolve #comment setting linebreaks to true for NoWhitespac…
…eBefore->dot and continuation indentation to be 2
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/chandnisingh/incubator-apex-core APEX-204
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-apex-core/pull/125.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 #125
----
commit d7afed510bc884e8f6beb0da5a5276036e5fd183
Author: Chandni Singh <cs...@apache.org>
Date: 2015-10-20T22:25:51Z
APEX-204 #resolve #comment setting linebreaks to true for NoWhitespaceBefore->dot and continuation indentation to be 2
----
---
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-204 #resolve #comment setti...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150063801
@chandnisingh please squash commits before merge
---
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-204 #resolve #comment setti...
Posted by tweise <gi...@git.apache.org>.
Github user tweise commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150065599
Nevermind, I see you piggyback another ticket. And I thought we are doing one event at a time...
---
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-204 #resolve #comment setti...
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/125#discussion_r42700482
--- Diff: apex_checks.xml ---
@@ -54,13 +54,13 @@
<module name="LeftCurly">
<property name="option" value="eol"/>
<property name="tokens"
- value="LITERAL_CATCH, LITERAL_DO, LITERAL_ELSE, LITERAL_FINALLY, LITERAL_FOR, LITERAL_IF, LITERAL_SWITCH, LITERAL_SYNCHRONIZED, LITERAL_TRY, LITERAL_WHILE"/>
+ value="LITERAL_CATCH, LITERAL_DO, LITERAL_ELSE, LITERAL_FINALLY, LITERAL_FOR, LITERAL_IF, LITERAL_SWITCH, LITERAL_SYNCHRONIZED, LITERAL_TRY, LITERAL_WHILE, STATIC_INIT"/>
--- End diff --
Is there a way to enforce brace on a new line for anonymous class definitions like in
```
Thread t = new Thread(new Runnable()
{ // brace on the new line is not enforced
@Override
public void run()
{
}
});
```
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/125#discussion_r42701377
--- Diff: engine/pom.xml ---
@@ -145,7 +145,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<configuration>
- <maxAllowedViolations>2320</maxAllowedViolations>
+ <maxAllowedViolations>2063</maxAllowedViolations>
--- End diff --
The violations were fixed as part of other commits which I don't know. Whoever fixed them didn't change this number
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-149973934
@ilganeli that is not an inconsistency. We adopted K&R style and the placement of braces is according to that.
https://en.wikipedia.org/wiki/Indent_style#K.26R_style
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150041997
@vrozov the other issue is not a checkstyle issue. It is a bug with checked in templates.
---
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-204 #resolve #comment setti...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150041822
@chandnisingh I'd prefer to address all known checkstyle issues in a single commit.
---
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-204 #resolve #comment setti...
Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-149980756
@chandnisingh Understood, either way, code style should be updated to reflect this.
---
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-204 #resolve #comment setti...
Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-149935494
@chandnisingh
I've identified another error:
Braces placement for other should be (next-line) to be consistent with braces placement for others and for style guide.
---
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-204 #resolve #comment setti...
Posted by tweise <gi...@git.apache.org>.
Github user tweise commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150057324
I think we can do multiple sweeps by area and a final one that catches everything left.
---
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-204 #resolve #comment setti...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150047628
I don't see option in IntelliJ to change brace location for static or synchronized. Both fall into other settings. I think we need to change code style and have static to be consistent with synchronized()
```
static {
...
}
synchronized() {
...
}
```
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-149981096
Sorry which code style are you referring to? :-)
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150057979
I checked Eclipse and Intellij and they don't differentiate between blocks within a class and method so I am going to push the change.
---
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-204 #resolve #comment setti...
Posted by tweise <gi...@git.apache.org>.
Github user tweise commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150058169
@vrozov Vlad, my interpretation of support of #1 is the desire to fix all issues in one go instead of sometime in the future, but not necessarily a single commit.
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150033031
@ilganeli @vrozov I have created another ticket for that issue
https://malhar.atlassian.net/browse/APEX-211
This pull request addresses the issues currently in APEX-204 and can be merged
---
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-204 #resolve #comment setti...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150053367
The code style is not in stone. It needs to be convenient for developers to follow and adhere. Both static and synchronized() designate block of code and it seems that IntelliJ formats both the same way and there is no separate setting for static and/or synchronized().
I can't find reference to K&R style regarding class/method scope. Here is reference to [google code style]( https://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style). Note that braces follow the Kernighan and Ritchie style ("Egyptian brackets") for nonempty blocks and **block-like constructs**:
* No line break before the opening brace.
* Line break after the opening brace.
* Line break before the closing brace.
* Line break after the closing brace if that brace terminates a statement or the body of a method, constructor or named class. For example, there is no line break after the brace if it is followed by else or a comma.
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150042702
@vrozov placement of block is according to K&R style and all IDEs support that.
---
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-204 #resolve #comment setti...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150030724
@ilganeli please update APEX-204 JIRA to add discrepancy for the static block.
@chandnisingh can you please check what is supported for static block in all major IDEs
---
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-204 #resolve #comment setti...
Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150056437
Vlad - I am concerned that if all check styles are a single commit it will be overwhelmingly large. I think one commit per small batch of files is more reasonable.
Thank you,
Ilya Ganelin
-----Original Message-----
From: Vlad Rozov [notifications@github.com<ma...@github.com>]
Sent: Wednesday, October 21, 2015 06:25 PM Eastern Standard Time
To: apache/incubator-apex-core
Cc: Ganelin, Ilya
Subject: Re: [incubator-apex-core] APEX-204 #resolve #comment setting linebreaks to true for NoWhitespac… (#125)
@chandnisingh<https://github.com/chandnisingh> I'd prefer to address all known checkstyle issues in a single commit.
—
Reply to this email directly or view it on GitHub<https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150041822>.
________________________________________________________
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.
________________________________________________________
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-204 #resolve #comment setti...
Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-149982896
Sorry, to be clear, in IntelliJ the apex-style format sets the following code:
``` static
{
...
...
}
```
As
``` static {
...
...
}
```.
The first is what's actually valid according to the 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-204 #resolve #comment setti...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-apex-core/pull/125
---
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-204 #resolve #comment setti...
Posted by tweise <gi...@git.apache.org>.
Github user tweise commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150065440
I see only one JIRA referenced? But in general, when you submit the good old patch to some other projects, that's a single commit. And in the main repository, we want to see the net change for a given issue and not the many different things that I might have tried in my repo to get there.
---
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-204 #resolve #comment setti...
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/125#discussion_r42700209
--- Diff: engine/pom.xml ---
@@ -145,7 +145,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<configuration>
- <maxAllowedViolations>2320</maxAllowedViolations>
+ <maxAllowedViolations>2063</maxAllowedViolations>
--- End diff --
Is the number reduced due to change to the apex_checks.xml or code style violations were fixed as part of other commits?
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150048520
@vrozov static blocks are defined within a class scope and from what I read about K&R style anything within a class scope should left brace placement in the next line.
Anything within a method scope should have left brace at the eol.
---
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-204 #resolve #comment setti...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150062597
@tweise Thomas, can you please bring this to e-mail thread on dev@apex.incubator.apache.org.
---
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-204 #resolve #comment setti...
Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150107202
@tweise @chandnisingh This looks good to merge, for future I agree with @tweise that we should try to avoid solving multiple issues with one PR where possible (granted I know that's my fault here).
---
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-204 #resolve #comment setti...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150065394
This was an existing practice. If we are going to change it, let's put it for the vote.
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150063010
@ilganeli Can you please merge this pull request. Thanks
---
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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150010897
got it
---
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-204 #resolve #comment setti...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150042439
@chandnisingh please check whether all IDEs support separate settings for static blocks, otherwise it may be necessary to adjust 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-204 #resolve #comment setti...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150057539
@ilganeli Ilya, sorry for not being clear, I mean that I prefer to fix checkstyle definition issues (apex_check.xml) in one single commit, it does not apply to .java files that have code style violations. There is a separate thread and JIRA filed to address existing large number of code style violations in .java where majority of community seems to prefer #1 option that means one single commit. I voted for #3.
---
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-204 #resolve
#comment setti...
Posted by Pramod Immaneni <pr...@datatorrent.com>.
I think additional commits from reviews should remain and not be
squashed as long as the number of commits is reasonable.
Thanks
> On Oct 21, 2015, at 5:42 PM, chandnisingh <gi...@git.apache.org> wrote:
>
> Github user chandnisingh commented on the pull request:
>
> https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150064224
>
> why? the 2 commits address 2 distinct issues then why should we squash them. Actually if you check other apache projects, it is recommended not to squash the commits when the change was a result of a review or another issue which was brought up.
>
>
> ---
> 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-204 #resolve #comment setti...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/125#issuecomment-150064224
why? the 2 commits address 2 distinct issues then why should we squash them. Actually if you check other apache projects, it is recommended not to squash the commits when the change was a result of a review or another issue which was brought up.
---
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.
---