gresockj commented on pull request #5369: URL: https://github.com/apache/nifi/pull/5369#issuecomment-931306105
> @gresockj this is great! This is a huge lift, to add a completely new type of extension point! I've not done any testing at all yet. But I've spent a good bit of time reviewing all of the code. Looks good for the most part - and you found _A LOT_ of minor typos in the javadocs - thanks for fixing those! :) > > Wanted to go ahead and submit my review, before jumping in to try it out, just because I feel like some of my thoughts may end up warranting some discussions. Especially around the notion of SensitiveParameterProvider vs NonSensitiveParameterProvider and those really becoming a single interface. I think this would yield a cleaner design. And we need to be really careful that we nail this down really well the first time because once we introduce something into `nifi-api` we can't really break backward compatibility until a 2.0 release comes, so let's take the time to make sure we're both in agreement on what makes the most sense there. > > Thanks again! Thanks for a thorough review, @markap14, this is very helpful! I believe I've addressed everything in the latest commit, except for collapsing Sensitive/NonSensitive ParameterProviders into one interface. I agree with your assessment, and will work on this in a separate commit since it will be a non-trivial change. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
