joewitt commented on code in PR #9600:
URL: https://github.com/apache/nifi/pull/9600#discussion_r1898626443
##########
nifi-extension-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java:
##########
@@ -139,23 +139,16 @@ abstract class AbstractAMQPProcessor<T extends
AMQPWorker> extends AbstractProce
.addValidator(StandardValidators.BOOLEAN_VALIDATOR)
.build();
- private static final List<PropertyDescriptor> propertyDescriptors;
-
- static {
- propertyDescriptors = List.of(
- BROKERS,
- HOST, PORT,
- V_HOST,
- USER,
- PASSWORD,
- AMQP_VERSION,
- SSL_CONTEXT_SERVICE,
- USE_CERT_AUTHENTICATION);
- }
-
- protected static List<PropertyDescriptor> getCommonPropertyDescriptors() {
- return propertyDescriptors;
- }
+ protected static final List<PropertyDescriptor> PARENT_PROPERTIES =
List.of(
+ BROKERS,
+ HOST, PORT,
+ V_HOST,
+ USER,
+ PASSWORD,
+ AMQP_VERSION,
+ SSL_CONTEXT_SERVICE,
+ USE_CERT_AUTHENTICATION
+ );
Review Comment:
Here the parent class 'getCommonPropertyDescriptors' method is removed and
instead a subclass is supposed to know to use the static list of descriptors as
they build their own lists. I think this weakens what the author intended in
building that class.
Also there is at least one example much later on in this PR where you kept
the parent method. I think keeping the parent method better conveys intent to
any subclassers.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]