On Mon, Jun 13, 2016 at 07:48:58PM +0200, Pavel Vomacka wrote:
> On 06/13/2016 06:55 AM, Fraser Tweedale wrote:
> > 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.
> Hello, thank you for review.
> > 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.
> The field removed.
> > 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).
> FIXME comment removed.
> > 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.
> I added tooltip next to the checkbox on adder dialog and also note above the
> table with CAs in CA ACL details view.
The message has a small typo: "specificed" should be "specified"
(the typo occurrs in both places).
Once this is fixed, ACK.
Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code