<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> </head> <body bgcolor="#ffffff" text="#000000"> On 11/06/2010 08:23 PM, Adam Young wrote: <blockquote cite="mid:4CD5F16B.1010501@redhat.com" type="cite"> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> On 11/06/2010 12:53 AM, Endi Sukma Dewata wrote: <blockquote cite="mid:4CD4DF34.7090901@redhat.com" type="cite">Hi, Please review the attached patch. Thanks! <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://fedorahosted.org/reviewboard/r/102/">https://fedorahosted.org/reviewboard/r/102/</a> IPA commands now can be defined in these classes: - ipa_command: a single IPA command - ipa_batch_command: a batch command for executing multiple commands on the server side using the new batch plugin The dialog boxes for adding and removing entries have been refactored: - ipa_dialog: base class for dialog boxes - ipa_adder_dialog: generic adder dialog box - ipa_deleter_dialog: generic deleter dialog box - ipa_association_adder_dialog: adding entity association - ipa_association_deleter_dialog: removing entity association Dialog boxes for adding/deleting HBAC users, hosts, services, and sourcehosts are implemented using the association dialog boxes. The dialog box for adding access time is implemented using ipa_dialog and currently contains only a text field. This will be replaced with a custom dialog box in a separate patch. The dialog box for removing access time is implemented using the generic deleter class because it's not an association. Removing multiple access times is implemented using batch operations. New test data files for access times have been added. <pre wrap=""><fieldset class="mimeAttachmentHeader"></fieldset> _______________________________________________ Freeipa-devel mailing list <a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a> <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre> </blockquote> The Time Dialog needs a format. I couldn't guess a value that it would accept. The retry/cancel message is really confusing, too. Tried 1200, 12:00 and 1200-1300. 1. Put in text showing the format. 2. Validate using a regex 3. Don't use the standard error handling, show the error inside the dialog box that has the time in it. <pre wrap=""> <fieldset class="mimeAttachmentHeader"></fieldset> _______________________________________________ Freeipa-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a> <a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre> </blockquote> The rest of the patch looks good. I think we can get away with just a comment on the time formate for this, get it checked in, and do the follow on work in another patch. </body> </html>