After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 585303 - port to PolicyKit 1.0
port to PolicyKit 1.0
Status: RESOLVED FIXED
Product: GConf
Classification: Deprecated
Component: gconf
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GConf Maintainers
GConf Maintainers
Depends on:
Blocks: 585596
 
 
Reported: 2009-06-10 03:44 UTC by Matthias Clasen
Modified: 2009-08-13 17:20 UTC
See Also:
GNOME target: 2.28.x
GNOME version: ---


Attachments
patch (41.72 KB, patch)
2009-06-10 03:44 UTC, Matthias Clasen
none Details | Review
improved patch (41.60 KB, patch)
2009-06-12 18:32 UTC, Matthias Clasen
reviewed Details | Review
patch against master (47.82 KB, patch)
2009-07-21 02:16 UTC, Matthias Clasen
committed Details | Review

Description Matthias Clasen 2009-06-10 03:44:23 UTC
Here is the patch that we use in Fedora.
Comment 1 Matthias Clasen 2009-06-10 03:44:49 UTC
Created attachment 136241 [details] [review]
patch
Comment 2 Matthias Clasen 2009-06-12 18:32:02 UTC
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.
Comment 3 Colin Walters 2009-07-15 16:47:31 UTC
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.
Comment 4 Colin Walters 2009-07-15 16:49:56 UTC
Also using "git format-patch" would be good.
Comment 5 Colin Walters 2009-07-15 16:57:15 UTC
This patch doesn't apply at all to git master...
Comment 6 Matthias Clasen 2009-07-21 02:16:32 UTC
Created attachment 138872 [details] [review]
patch against master
Comment 7 André Klapper 2009-08-12 10:23:28 UTC
Can the patch please get in very soon so this receives some testing for 2.28?