exceptionfactory commented on code in PR #10653:
URL: https://github.com/apache/nifi/pull/10653#discussion_r2635294273
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java:
##########
@@ -45,15 +45,15 @@
public abstract class AbstractS3Processor extends
AbstractAwsSyncProcessor<S3Client, S3ClientBuilderWrapper> {
// Obsolete property names
+ public static final String OBSOLETE_SIGNER_OVERRIDE = "Signer Override";
+ public static final String OBSOLETE_CUSTOM_SIGNER_CLASS_NAME_1 =
"custom-signer-class-name";
+ public static final String OBSOLETE_CUSTOM_SIGNER_CLASS_NAME_2 = "Custom
Signer Class Name";
+ public static final String OBSOLETE_CUSTOM_SIGNER_MODULE_LOCATION_1 =
"custom-signer-module-location";
+ public static final String OBSOLETE_CUSTOM_SIGNER_MODULE_LOCATION_2 =
"Custom Signer Module Location";
Review Comment:
That's a fair point, I realize my comment sounds odd from another
perspective. What I meant by that was having the public static variables means
that those elements can be referenced by any number of potentially-unrelated
classes. To your point, finding references using the variables is easier in any
IDE, versus a general string search. The more fundamental concern I had in mind
was what I mentioned elsewhere, about adding public variables solely to support
tests for migration methods. I also realize this particular issue is a bit
messy due to the inheritance tree of the `AbstractS3Processor`. All that to
say, I think avoiding the change in this particular case is marginally better,
though not perfect either. Thanks for making the adjustments.
--
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]
