Pavel Zuna wrote:
Pavel Zuna wrote:Rob Crittenden wrote:I'm sending an updated version of all the patches. They should apply on the current master. I think they should now address most of your concerns and I also fixed some bugs I found on my own.PavelSome are good to go, others need some more work: 0001-config: ack 0002-group: ack 0003-hostgroups: nack, hostgroups-show doesn't show members by default.Fixed in attached patch.0004-netgroup: nack, fails a ton of unit testsFixed in patch I sent previously today.0005-rolgroup: nack, rolegroup-find doesn't work (try serviceadmin). rolegroup-show doesn't include members by default.Both "member thing" fixed, but *-find seems to work for me. Can you post the exact commands that fail?0006-taskgroup: nack, taskgroup-find doesn't work (try ipa addhosts) taskgroup-show doesn't include members by default.0007-tests: ack though we should do some non-raw tests as well IMHO. In test-hostgroup looks like there is some leftover debug output that should be removed before committing.Removed the leftover debug output in attached patch.0008-pwpolicy: nack, have another patch floating around that covers most of this. I'll try to incorporate some of your ideas into my patch once committed.Ok.Not missing, just wrong numbering. There was an unrelated patch that got in between the plugin patches.0009 missing?0010-service: ack, but we should try to avoid shadowing built-in names (filter)Sometimes it might be justified, but in this case we really should avoid it, because it's actually in the base class and what if someone wants to use python's filter function inside the callback? (They would have to rename the param or do some hacking around.) It doesn't hurt anything for now, but this will have to be patched at several places - I'll post a patch on Monday.0011-user: ack but changes like this seem to make it less readable: - textui.print_dashed('Unlocked user "%s".' % uid) + textui.print_dashed('Unlocked user "%s".' % keys[-1])I realize that *keys is more generic than specifying one argument but there is no documentation saying was keys means.I'm really slow at documenting things, but it's in the works.Some general observations that don't necessarily need to be addressed here but do need to be done:- adding/removing member failures should return non-zero return code (so you can test $? on cmd-line) - add/remove-member should probably prompt for things if not provided on command-line (and allow empty values). Right now if you do an add-member it prompts for group to modify and then submits an empty requestOk, I'll try to address these issues as soon as possible.The user and *group plugins should be pushed at once. Everything else should be safe individually.Let me know if it is safe to push these patches individually.robPavelAwwww. Forgot to attach the patches... Pavel
Pushed patch 0001 and 0002. Patch 0003 does not apply rob
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
