[Freeipa-devel] [PATCH] 0270-removing-setters-setup-and-init

[Adam Young]

On 07/20/2011 08:08 PM, Endi Sukma Dewata wrote:
> On 7/20/2011 2:50 PM, Adam Young wrote:
>
>>> 8. Triggering a stack trace by calling null function probably will
>>> only work with Firebug, normal users will not get any notification
>>> about the error. This happens in dialog.js:301 and widget.js:1137.
>
>> Gonna leave this, as we will catch things in development, and it won't
>> happen on the live servers. These are "never reach" type conditions.
>
> Would it be better to throw an exception instead?

Possibly.  This code is going to get caught by the catch block in ipa.js 
get_entity anyway, so there is not much difference.
>
> Still not done, but here are new findings:
>
> 12. The search filter doesn't work initially. Reload the UI main page, 
> (make sure there's no URL parameters), enter a filter, then hit Enter 
> or click the icon, there's nothing happening. Go to another tab, then 
> come back to the main page. Now the filter will work.
>

Fixed.  Was a pre existing problem in navigation.js,  around line 115

> 13. The 'other_entity' still contains entity name instead of entity 
> object. One solution for the circular dependency problem is to create 
> all entity objects first, then create the facets & dialogs in the 
> second stage. This requires simple modification to the entity_factories.

Not sure I want to make that change in this patch, though.  Circular 
dependencies would still be tricky to resolve, and the initialization 
code would be more complicated than it is now.
>
> 14. The comment "move into the table_widget" on association.js:710 
> might not be correct. I think we should try to reuse 
> IPA.association_table_widget inside IPA.association_facet.

Agreed. Comment changed
>
> 15. The assignment on association.js:733 is unused:
>
>   var entity = that.entity;
Gone
>
> 16. Commented code in details.js:459 can be deleted.
Gone
>
> 17. The code in dialog.js lines 489 and 496 can be combined into:
>
>   that.external_field = $('<input/>', {
>       ...
>   }).appendTo(external_panel);
Done
>
> 18. Since the layout support is dropped, the install/ui/layouts can be 
> removed as well.
>

Doing this in another patch.  It involves changes to the build process.  
I've stareted it, but it isn't ready to be posted for review.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-admiyo-0270-4-removing-setters-setup-and-init.patch
Type: text/x-patch
Size: 188638 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110720/d54e0cf9/attachment.bin>