On 06/17/2014 12:25 PM, Tomas Babej wrote:

On 05/26/2014 06:20 PM, Petr Viktorin wrote:
On 05/20/2014 06:15 PM, Tomas Babej wrote:
Hi,

the following set of patches fixes:

https://fedorahosted.org/freeipa/ticket/4274
https://fedorahosted.org/freeipa/ticket/4263
https://fedorahosted.org/freeipa/ticket/4324
https://fedorahosted.org/freeipa/ticket/4340
https://fedorahosted.org/freeipa/ticket/4341

and additional minor issues.

The improvemed CI test coverage for the sudorule plugin is added as a
bonus.

You've dropped most of the long commit messages and ticket URLs. Why?


0187: OK

(Speaking of PEP8, if you could remove the baseldap star import from
sudorule.py, it would be great...)


This one did hurt, but the star disappeared.

Thank you, much appreciated.
(Especially the fact that Int is no longer imported from baseldap)

General thoughts:

Would it be possible to merge schema_compat.uldif and
install/updates/10-schema_compat.update into one file? Is the uldif
special somehow? I guess this is a question for Rob.
It would be nice to add a link to some schema-compat-entry-attribute
documentation to these files.

I added Rob to cc. Rob, can you elaborate on this?


0188 - sudorule: Allow using hostmasks for setting allowed hosts

If I run sudorule-add-host / sudorule-remove-host with a hostmask, but not host/hostgroup, I get prompted for host and hostgroup. I don't think that's the intended behavior.

0189: OK
0190: OK
0191: OK
0192: OK

0193 sudorule: Make sure all the relevant attributes are checked when
setting category to ALL

You're missing the `_` for the hostcategory error message.
Did you think about using something like _("%s cannot be set to 'all'
while there are %s")?

Fixed. Initially, I changed the message as you suggested, but then I
realized, that this might pose a problem for translations that do not
follow the word order in the sentence as it is defined in English language.

Right, sorry for the incorrect example. You can use named substitutions for that: _("can't %(action)s while %(state)s") % {'action': 'move', 'state': 'asleep'}

One more thing - the function is only called once, could you move it to the for loop?

0194: OK
0195: OK
0196-0198: OK
0199-0201: OK

0225:
Looks good. Could you also document the arguments & return value in *_external_post_callback docstrings?


--
PetrĀ³

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

Reply via email to