[GitHub] nifi issue #2034: NIFI-4215 Fixed stackoverflow error when NiFi tries to par...

2017-07-30 Thread jvwing
Github user jvwing commented on the issue:

https://github.com/apache/nifi/pull/2034
  
Looks good.  Thanks again for fixing this, @Wesley-Lawrence.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi issue #2034: NIFI-4215 Fixed stackoverflow error when NiFi tries to par...

2017-07-30 Thread Wesley-Lawrence
Github user Wesley-Lawrence commented on the issue:

https://github.com/apache/nifi/pull/2034
  
Alright, changes made, and contrib-check still passing for me.

Thanks for the review @jvwing!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi issue #2034: NIFI-4215 Fixed stackoverflow error when NiFi tries to par...

2017-07-29 Thread jvwing
Github user jvwing commented on the issue:

https://github.com/apache/nifi/pull/2034
  
@Wesley-Lawrence  I think this is a good fix, your approach to the solution 
seems solid.  Thanks for adding the unit tests for the recursive and 
mutually-referential cases.  Would you please:

1. Fix the checkstyle issue and remove the change to the checkstyle 
definitions
1. Optionally change the foundSchemas/knownRecordTypes exception message
1. Squash and rebase on master




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi issue #2034: NIFI-4215 Fixed stackoverflow error when NiFi tries to par...

2017-07-26 Thread jvwing
Github user jvwing commented on the issue:

https://github.com/apache/nifi/pull/2034
  
Thanks @pvillard31!

@Wesley-Lawrence Don't worry about squashing yet, we can do that as a final 
step.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi issue #2034: NIFI-4215 Fixed stackoverflow error when NiFi tries to par...

2017-07-26 Thread Wesley-Lawrence
Github user Wesley-Lawrence commented on the issue:

https://github.com/apache/nifi/pull/2034
  
@pvillard31 Yup, you're right, that solves the issue for me 😃.

I got so caught up in the second `RightCurly` definitons saying `}` should 
be alone, and other `if`s being that way, I didn't see the correct style for 
`if ... else`s.

I'll fix that line, re-add the original `RightCurly` definition back in, 
and push a new squashed commit.

Thanks again, both of you.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi issue #2034: NIFI-4215 Fixed stackoverflow error when NiFi tries to par...

2017-07-26 Thread pvillard31
Github user pvillard31 commented on the issue:

https://github.com/apache/nifi/pull/2034
  
@Wesley-Lawrence I believe this should be:

java
if() {

} else {

}



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi issue #2034: NIFI-4215 Fixed stackoverflow error when NiFi tries to par...

2017-07-26 Thread Wesley-Lawrence
Github user Wesley-Lawrence commented on the issue:

https://github.com/apache/nifi/pull/2034
  
Thanks for taking a look @jvwing!

I didn't want to change it, but I keep getting the following error with it;
```
[ERROR] Failed to execute goal 
org.apache.maven.plugins:maven-checkstyle-plugin:2.15:check (check-style) on 
project nifi-avro-record-utils: You have 1 Checkstyle violation. -> [Help 1]
```

Earlier in the maven log;
```
[INFO] --- maven-checkstyle-plugin:2.15:check (check-style) @ 
nifi-avro-record-utils ---
[WARNING] src/main/java/org/apache/nifi/avro/AvroTypeUtil.java[275:17] 
(blocks) RightCurly: '}' should be on the same line.
```

Which references a `}` I added here;
```
273if (knownRecordTypes.containsKey(schemaFullName)) {
274return knownRecordTypes.get(schemaFullName);
275--> }
276else {
```

However, this is the style used everywhere in NiFi, and is the one defined 
by the `RightCurly` section below the one I removed.

I suspect it's something weird in my environment, but removing the default 
`RightCurly` definition fixed my issue, and it looks like it was just left over 
from some old migration, so I figured it could be safely removed.

Out of curiosity, if you run a contrib check, do you get the same error I 
do?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi issue #2034: NIFI-4215 Fixed stackoverflow error when NiFi tries to par...

2017-07-25 Thread jvwing
Github user jvwing commented on the issue:

https://github.com/apache/nifi/pull/2034
  
@Wesley-Lawrence Thanks for putting this PR together.  I am still reviewing 
- the only immediate feedback I can give is that I would prefer not to update 
the checkstyle definition in pom.xml as part of this fix for the 
AvroSchemaRegistry issue.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi issue #2034: NIFI-4215 Fixed stackoverflow error when NiFi tries to par...

2017-07-25 Thread jvwing
Github user jvwing commented on the issue:

https://github.com/apache/nifi/pull/2034
  
Reviewing...


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---