On Fri, Jun 10, 2016 at 04:34:33PM +0200, Pavel Vomacka wrote: > Hello, > > please review these new patches which add WebUI for Sub-CAs. > > https://fedorahosted.org/freeipa/ticket/5939 > Hi Pavel, I have reviewed the functionality of the patches. Functionality is good - a few minor comments below.
Patch 45: 1) In the main `Certificate Authorities' table, `Subject DN' is showing the DN of the IPA object, instead of the Subject DN. 2) In the `Certificate Authorities' detail table, there is an unlabelled row showing the DN of the IPA object. IMO we do not need to show this value at all. Patch 46: 3) I see a FIXME in certificate.js. The behaviour (default to IPA CA / 'ipa') is OK. Alternatively, you could allow the user to not specify a CA (this will allow the default - currently 'ipa' - to be controlled by server). Patch 47: 4) For backwards compatibility, a CA ACL without any specified CAs (and not cacat=all) implies the 'ipa' CA. It would be good to indicate this in the UI somehow, or include a notice to explain. Cheers, Fraser -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code