rgodfrey edited a comment on pull request #48: URL: https://github.com/apache/qpid-broker-j/pull/48#issuecomment-658205421
@gemmellr has already commented on the dependency issue. From my perspective, the majority of this change seems to have little to do with adding Graylog support, but is instead a modification to add a new mechanism for validating attributes of configured objects. I think this should be separated from the changes to add (optional) support for graylog. Custom validation has, up until now, been implemented by overriding the onValidate(), validateChange() and validateOnCreate() methods of AbstractConfiguredObject in the derived class. While this is a bit messy, I would suggest that you first submit a change using this mechanism; and then submit a proposal for adding the validator as an option to attributes, which we can then discuss (I'm not sure I'm 100% in agreement with the proposed implementation in this PR, but, as above, I thing we should separate that from the logging support) ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
