Jan Cholasta wrote:
> Dne 28.7.2014 v 21:39 Rob Crittenden napsal(a):
>> This is oh-so close. AFAICT it generally does what it should, I think it
>> is ready for a wider audience. Just a few more things:
>> 306: A while True loop is used for something which AFAICT can only ever
>> execute once. I'd think something like this is more readable:
>> for ca_nick, ca_flags in db.list_certs():
>> if db.has_nickname(ca_cert):
>> except ipautil.CalledProcessError:
>> "Failed to remove certificate %s" % ca_nick)
> Actually the while loop is necessary, because certutil -D (and in turn
> CertDB.delete_cert) deletes just a single cert with the nickname, but
> there may be more versions of it and we need to delete them all.
Aha, excellent point. This would be a great comment in the code!
>> +1 on the additional syslogs. It will help figure out what's going on if
>> things go sideways.
>> Otherwise things seem to be working. I think that fixing the above is
>> enough for a +57 with the promise of unit tests to back up some of these
>> new functions.
> I'm working on that.
> I have made a slight adjustment to patch 246 because of
> <https://fedorahosted.org/freeipa/ticket/4039>, see
> Updated rebased patches attached.
> (once again, the correct order to apply them is 241-253, 262-294,
> 303-305, 295-299, 306-307)
ACK on 246.
IMHO this is ready to be pushed if you can add the comment on 306.
A slight rebase on 251 is needed for freeipa.spec.in.
Freeipa-devel mailing list