[Freeipa-devel] [PATCH] 204 Added sudo options.

[Adam Young]

On 07/11/2011 02:46 PM, Endi Sukma Dewata wrote:
> On 7/11/2011 12:41 PM, Adam Young wrote:
>> Tempted to ACK. Before I do, question: why did you make it a section as
>> opposed to a widget? The only other place we have a custom section is
>> for the permissions, where we are optionally showing a set of related
>> widgets together, and we needed to reuse that logic between both the
>> facet and the adder dialog. Neither case applies here. The only benefit
>> I can see here is that it avoids the label. I suspect that the code
>> should be written as a custom widget, not as a section. However, written
>> this way is not such a major change from elsewhere that it is really
>> going to confust people, so I won't NACK it on that alone.
>
> The sudo options table is actually written using a custom widget, but 
> instead of creating a subclass, it's done by customizing an instance 
> of table widget. I'm trying to avoid creating single-use classes that 
> are too low level.
>
> We actually have a number of custom sections in sudo and HBAC. They 
> are mainly used for code organization. Without them the details facet 
> will become too long or complex.
>
> Also for consistency, we usually use tables in their full width and 
> without labels. One exception is the services table in HBAC service 
> group, but the label is actually redundant. Tables in our UI usually 
> have their own sections, they are never put in the same section with 
> other text widgets. We can always change the layout if UXD has a 
> different design.
>
OK, that makes sense.  ACK