On Mon, 25 Jun 2012, Endi Sukma Dewata wrote:
On 6/25/2012 10:33 AM, Petr Vobornik wrote:
On 06/25/2012 04:52 PM, Petr Vobornik wrote:
On 06/25/2012 04:37 PM, Alexander Bokovoy wrote:
On Mon, 25 Jun 2012, Alexander Bokovoy wrote:
On Mon, 25 Jun 2012, Simo Sorce wrote:
On Mon, 2012-06-25 at 12:43 +0200, Petr Vobornik wrote:
On 06/23/2012 01:44 AM, Endi Sukma Dewata wrote:
On 6/22/2012 11:48 AM, Alexander Bokovoy wrote:
2. First two chunks of install/ui/test/data/ipa_init_commands.json
and
install/ui/test/data/ipa_init_objects.json changes look
unrelated to
this ticket.
These files are snapshots of metadata used for demo/testing. I
suppose
Petr was updating the entire files which automatically includes
recent
changes to the metadata.
ACK
Ditto. The UI code looks fine so it can be pushed. Btw, nice use of
layout class.
Some comments:
1. The CLI command to add trust is trust-add-ad. Should the UI
button
also say "Add AD"? If we later support additional trust types would
that
appear as separate buttons/dialogs or same button/dialog with maybe
drop-down list to select the type?
"Add AD" label seems weird to me. Now we support only one type of
trust.
We should keep the 'Add'.
I have to say I also find the trust-add-ad command really weird,
difficult to use and to spell vaocally and to remember.
Alexander can we change it to trust-add --type=ad
where we can omit --type=ad for now as it is the only one, later on we
can decide what to default to when --type is omitted.
Patch attached (not tested).
Attached is tested patch.
ACK abbra-53 & abbra-54. One thing though, the error message is not
very user friendly. Feel free to fix before push.
% ipa trust-add ad.test --type=asdf
ipa: ERROR: invalid 'type': must be one of (u'ad',)
The ValidationError specifies this message 'only "ad" is supported'
but it doesn't appear in the error message above.
The message above comes from StrEnum() validator which is common one for
all StrEnum()s. I made a ValidationError in execute() method to catch up
any discrepancies when other types of trust will be added as Python
doesn't have 'case/switch' so you are left alone with 'if' or list-based
lambdas which don't look so clear.
What we probably want to add is more friendly way to display these
StrEnum values, dropping u'' and simply showing proper unicode as we do
for help already.
Endi, could you please file a minor bug for it?
--
/ Alexander Bokovoy
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
