Everything looks good to me. +1 On Thu, 2013-11-28 at 12:18 +0100, Petr Viktorin wrote: > Thanks! > Just a bit of cleaning up now, sending a patch with proposed changes to > speed things up. > > Patch needs a tiny rebase. > Points I missed: > - There are some unused imports. > - ValidationError takes the attribute name in `name` rather than the > name of the CLI option. > > >> Now the validation is too strict, a port is not accepted. > > > > Fixed. > > "invalid!" is pretty bad for an error message. I put it in as a > placeholder, but I wasn't clear about that, sorry! > > >> Should non-FQDN hostnames be allowed? > > > > I agree they should not. Fixed. > > validate_hostname() has a check_fqdn argument, no need to do this manually. > > >>>> ipatokenusermapattribute is also not validated. Not sure if it needs to > >>>> be. > >>> > >>> I don't think validation is really possible outside of the permitted > >>> characters for an LDAP attribute. > >> > >> I think if "$%^&*" is allowed, we'll get a bug from QA soon enough. > > > > Fixed. > > The `sre` module is named `re` since Python 2.5. > > >> We generally output lists; this should also be a list with one element. > > > > Fixed. > > > >> Attaching updated tests. > > > > A few of these tests are still failing for me, but it is not immediately > > obvious why. They seem to be getting answers from previous queries. I'm > > not sure if this is something wrong with my code or the tests. Can you > > take a look at it? > > My bad, I've used a wrong variable name. >
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
