heitzjm commented on PR #4474: URL: https://github.com/apache/solr/pull/4474#issuecomment-5007911098
> Left a few comments/questions inline, but this looks close enough that I hope we can merge it soon. I'll circle back next week for a re-review if I see replies here. > > One more general comment: > > I'm surprised that this PR can change the attribute-name used without needing any corresponding changes in CertAuthPluginTest. Should there be some changes there do you think? CertAuthPluginTest.java uses a mock that does not take care of the looked up attribute, and provides unit tests. As the problem is an integration concern, I think it is fair that the unit tests did not detect the problem. However I am not convinced that writing an integration test would be worth the time... -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
