GNOME Bugzilla – Bug 585303
port to PolicyKit 1.0
Last modified: 2009-08-13 17:20:51 UTC
Here is the patch that we use in Fedora.
Created attachment 136241 [details] [review] patch
Created attachment 136464 [details] [review] improved patch The improved patch returns a tristate (0 - not allowed, 1 - challenge, 2 - allowed) instead of a simple boolean in the CanSet... calls.
I really don't like all the spurious whitespace changes. I know trailing whitespace is bad, but it's better to incrementally remove it, otherwise it makes diff review and merging problematic. Would you consider redoing the patch without the extra whitespace removal? I had a script to do this lying around based on the git diff but lost it, I can probably rewrite it though if you want. Only thing I spotted is: inside actions_ready_cb, why do we call stop_operation () in the error path, but not in the main path? Without knowing the PK api at all and not knowing this code in general it's hard to give a full review, but I don't see anything else obviously wrong. We should probably push this patch and iterate from there if anything else needs to be fixed.
Also using "git format-patch" would be good.
This patch doesn't apply at all to git master...
Created attachment 138872 [details] [review] patch against master
Can the patch please get in very soon so this receives some testing for 2.28?