MINIFICPP-1232 - PublishKafka processor doesn't validate some properties#794
MINIFICPP-1232 - PublishKafka processor doesn't validate some properties#794adam-markovics wants to merge 2 commits intoapache:masterfrom
Conversation
| core::Property PublishKafka::QueueBufferMaxMessage("Queue Max Message", "Maximum number of messages allowed on the producer queue", ""); | ||
| core::Property PublishKafka::CompressCodec("Compress Codec", "compression codec to use for compressing message sets", COMPRESSION_CODEC_NONE); | ||
|
|
||
| const core::Property PublishKafka::QueueBufferMaxTime( |
There was a problem hiding this comment.
to nitpick: with method chaining I don't know what the style guideline is, but I prefer the operator on the next line (and it seems that other properties do it like this, in this file)
There was a problem hiding this comment.
+1.
That's the way linter accepts it as well, so please change as @adamdebreceni suggest!
Unfortunately this extension is not linted. Yet. :(
There was a problem hiding this comment.
I like it this way too, so 2/2. Arpad, are you sure this is not accepted by the linter? I see no warnings when trying to add these lines to a linted file.
There was a problem hiding this comment.
Not sure if this still applies after your linter changes, but previously the linter only accepted the "->" operators to be at the begging of the new line.
If you take a look at standard processors, all use the other format.
There was a problem hiding this comment.
Done in new commit.
arpadboda
left a comment
arpadboda
left a comment
There was a problem hiding this comment.
Looks good, two minor comments.
| core::Property PublishKafka::QueueBufferMaxMessage("Queue Max Message", "Maximum number of messages allowed on the producer queue", ""); | ||
| core::Property PublishKafka::CompressCodec("Compress Codec", "compression codec to use for compressing message sets", COMPRESSION_CODEC_NONE); | ||
|
|
||
| const core::Property PublishKafka::QueueBufferMaxTime( |
There was a problem hiding this comment.
+1.
That's the way linter accepts it as well, so please change as @adamdebreceni suggest!
Unfortunately this extension is not linted. Yet. :(
| static const core::Property QueueBufferMaxTime; | ||
| static const core::Property QueueBufferMaxSize; | ||
| static const core::Property QueueBufferMaxMessage; | ||
| static const core::Property CompressCodec; |
There was a problem hiding this comment.
Done in new commit.
4 participants
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.