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 fe
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
poss
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 @@
org.apache.maven.plugins
maven-checkstyle-plugin
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 thi
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
repositor
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 a
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 wrote:
>
> Github user chandnisingh commented on the pull request:
>
>
> https://github.com/apache/incubator-apex-core/
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
squ
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 w
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 @@
+value="LITERAL_CATCH, LITERAL_DO, LITERAL_
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 @@
org.apache.maven.plugins
maven-checkstyle-plugin
-
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
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 em
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 c
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 p
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
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 a
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
re
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
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 i
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
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
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 pr
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
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 h
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 th
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
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 t
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
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 GitH
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
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/Inden
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 an
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
34 matches
Mail list logo
