[GitHub] [nifi] bejancsaba commented on pull request #5648: NIFI-9428: Extending OperandType to Enum

[GitBox <gi...@apache.org>]

bejancsaba commented on pull request #5648:
URL: https://github.com/apache/nifi/pull/5648#issuecomment-1008137872


   > Thanks for the contribution @bejancsaba! The `OperandType` enum license header does not meet standard Apache License 2.0 guidelines, so that needs to be corrected before this PR can be approved. Running Maven build with the `contrib-check` profile should indicate whether the header matches the expected license terms.
   > 
   > Other maintainers may have more substantive comments, but this change appears to make the `operand` property more restrictive, as opposed to the current `String` type. Will that restriction cause any problems for different versions of the C2 protocol?
   
   Thank you for the guidance. I corrected the License issue and validated my change with contrib-check. Following up on the comment from @mattyb149 I'm closing this PR and opening a new one against main under a new dedicated ticket and with the change requested here.
   
   As the [specification](https://cwiki.apache.org/confluence/display/MINIFI/C2+Design) is pretty specific about operation's and their valid operand(s) the idea was that with this deliberate restriction you could have "safer" implementations. I think we should be good with different c2 versions but I would appreciate if @kevdoran or @mattyb149 would take a look as they have more background and context here. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org