On 02/17/2011 05:21 AM, Martin Kosek wrote:
On Wed, 2011-02-16 at 10:46 -0500, Adam Young wrote:
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

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.
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.

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.

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.


Freeipa-devel mailing list

Looks good. Only problem is on braces. we have a code standard that is like this

IPA.something = function () {


IPA.something = function ()

This is due to Javascript being ambiguous in certain circumstances about where it puts an implicit end of statement.


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?

Freeipa-devel mailing list

Reply via email to