[Freeipa-devel] [PATCH] 0229-automount-delete-key

[Adam Young]

https://fedorahosted.org/freeipa/ticket/204


Comments for what was fixed are in the patch.  Here's what I didn't change:

> 4. Clicking 'Back to List' when viewing a map brings you back to list
>    of locations. Is this still intentional? Perhaps the label should be
>    changed to 'Back to Locations' or simply hidden.

Left it as is.  if UXD objects, we can change in a follow on patch.

>
> 5. The conditional fields in IPA.dialog are a little bit limited
>    because there is only one set of conditional fields which has to be
>    enabled/disabled together. It might be better to replace the
>    'conditional' boolean paramter into 'field_group' then replace the
>    enable/disable methods to accept a field group. This could be done
>    later.
Agreed.  I'd like to merge this with the sections code used for aci

> 8. In dialog.js line 626 and search.js line 253, the hasOwnProperty()
>    invocations are probably redundant because the key is obtained from
>    the object itself, so that method will always return true.
This falls under the rules from "Javascript the good parts" and is 
probably a good idea to leave in, even though in our code it is strictly 
unnecessary.

>
> 10. The 3rd level tab for automount key was removed. At this point does
>     it makes sense to remove the 3rd level tabs completely?
The 3rd tab will come back if/when we do autmountkey details.  Leaving 
for now.

>
> 11. The option values for automount map adder dialog could be
>     simplified to "direct" and "indirect".
>

The values used are what is appended to the command's method. Had to 
leave them as is to keep that working.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-admiyo-0229-5-automount-delete-key.patch
Type: text/x-patch
Size: 12081 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110601/486263b5/attachment.bin>