<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <meta content="text/html; charset=UTF-8" http-equiv="Content-Type"> </head> <body bgcolor="#ffffff" text="#000000"> On 02/17/2011 05:21 AM, Martin Kosek wrote: <blockquote cite="mid:1297938096.18411.48.camel@dhcp-25-52.brq.redhat.com" type="cite"> <pre wrap="">On Wed, 2011-02-16 at 10:46 -0500, Adam Young wrote: </pre> <blockquote type="cite"> <pre wrap=""> Almost there. I'd like to pull the sudo namespace out of ipa.js and put it into sudorule.js, then indicate that the other sudo files depend on sudo rule. I guess I should have been clearer: stuff like facets and widgets don't need to go into a sub, namespace, just custom code called by them. I'm thinking that widgets and facets in the long term should become a sub-namespace of IPA themselseves: so IPA.widget.text, IPA.facet.details, and then the more specific ones. While I don't want to do that in this patch, keep that in mind when deciding which namespace to put something into. A good rul of thumb is that an entity name should not be repeated in a function name, so something like IPA.sudo.sudorule_details_facet should be IPA.sudorule_details_facet but any custom functions it calls should be in IPA.sudo. </pre> </blockquote> <pre wrap=""> I have prepared a next version of patch with the above comments applied. Facets and widgets are in IPA namespace now. Still, I cannot do much of a renaming with sub-namespace custom methods that are called by *_widget or *_facet functions - they would collide. E.g. IPA.sudo.sudorule_add_dialog cannot be renamed to IPA.sudo.add_dialog because it would collide with renamed IPA.sudo.sudocmd_add_dialog. </pre> <blockquote type="cite"> <pre wrap=""> I'm being a bit picky here as this is probably the last major cleanup we'll get to do before GA, and this is the code that people will look at. I want it to be as understandable as possible. </pre> </blockquote> <pre wrap=""> I know that since you have worked on WebUI for a long time, you have a pretty clear picture what it should look like. I hope this patch version is consistent with the plan. Martin </pre> <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> Looks good. Only problem is on braces. we have a code standard that is like this IPA.something = function () { not IPA.something = function () { This is due to Javascript being ambiguous in certain circumstances about where it puts an implicit end of statement. <a class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/wiki/Javascript_Coding_Standards">https://fedorahosted.org/freeipa/wiki/Javascript_Coding_Standards</a> For name shortening, sudo.sudorule_ should be sudo.rule_ On the patch I sent you as an example, I broke the "View Cert" button. I didn't test that here. Did you make sure that still works? </body> </html>