The patches work, but I have some comments:

1. Would it be better to use radio buttons to define mutually exclusive options? Something like this:

  Group type: (o) Normal
              ( ) External
              ( ) POSIX
                  GID: [            ]

2. If you decide to use radio buttons for the adder dialog you might want to use similar layout in the details page.

3. Minor thing, the "posix" in "Change to posix group" probably should be capitalized to be consistent with the "POSIX group" field label. Feel free to fix before push.

4. I don't have the environment to test patch #192 but the code looks fine.

5. I noticed this problem, but I couldn't reproduce it anymore. When I specified a GID in the group adder dialog, the value seemed to be ignored, so the GID would always be generated. Also the GID wasn't validated. Probably just a glitch.

Endi S. Dewata

Freeipa-devel mailing list

Reply via email to