Github user joewitt commented on the issue:
https://github.com/apache/nifi/pull/2028
Bill,
Some feedback after reviewing in more detail.
1. There are no unit tests. I can see why they'd be non-portable given it
is executing a process but it does seem like a good candidate for an
integration-test at least. We should definitely have something. We do have a
naming convention for integration tests to execute.
2. The nar is not connected to the nifi-nar-bundles pom
3. The nar is not connected to the nifi-assembly. This is perfectly fine
though if you dont want the nar to be available in the build by default but
rather something someone opts into. I'm ok either way just pointing this out.
4. The L&N updates within the nar are a good start but are incomplete with
regard to the totality of the jars present within the nar.
5. The L&N updates in the nar will also need to be validated to see if any
need to be carried forward into the nifi-assembly nar.
6. There are unused imports which cause checkstyle/rat failures which is
why travis failed on all builds.
7. I'm not an expert on morphlines but it seems like it could be used to do
dangerous things on the filesystem. We have a RestrictedComponent annotation
for this purpose so it is probably best to add that to the processor.
8. The processor name for a 'users point of view' should probably be
'ExecuteMorphline'
9. You'll probably want to provide some documentation/additional
documentation as many other processors do helping the user (and reviewer to
evaluate).
10. Can you update to latest nifi version which is 1.6.0-SNAPSHOT in your
next commit. Not a big deal as I can do that easily enough but it helps.
This looks like a lot but for the L&N we have tons of examples. The rest
are pretty straightforward to sort out.
Let me know if you plan to tackle these.
Thanks
---
