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.

Attached updated UI patch.

ACK pvoborni-161-2. One thing also, the test data files would need to be updated because of the command rename. Feel free to fix before push.

--
Endi S. Dewata


_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to