Bug 454329

Summary: Review Request: PolicyKit-olpc - OLPC-specific PolicyKit overrides
Product: [Fedora] Fedora Reporter: Daniel Drake <dsd>
Component: Package ReviewAssignee: Rahul Sundaram <sundaram>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: christoph.wickert, dennis, fedora-package-review, notting, smohan
Target Milestone: ---Flags: sundaram: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 1.2-1.fc9 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-08-12 14:51:44 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Daniel Drake 2008-07-07 18:31:02 UTC
Spec URL: http://dev.laptop.org/~dsd/PolicyKit-olpc/
SRPM URL: http://dev.laptop.org/~dsd/PolicyKit-olpc/PolicyKit-olpc-1.0-1.fc9.src.rpm
Description: 
Software for the XO laptop requires some modification to the default policies
installed by certain applications. This package provides OLPC-specific
policy overrides. See http://dev.laptop.org/ticket/7350

This is my first package, and I am seeking a sponsor. Maybe Dennis Gilmore ;-)

Comment 1 Daniel Drake 2008-07-07 18:32:02 UTC
Spec URL: http://dev.laptop.org/~dsd/PolicyKit-olpc/PolicyKit-olpc.spec
(Sorry!)

Comment 2 Rahul Sundaram 2008-07-30 15:37:45 UTC
I can't verify functionality but it seems better to add a package dependency
instead of the file dependency on /var/lib/PolicyKit-public. Afaik, permissive
licenses like MIT or revised 2 clause BSD licenses are considered better from
the legal standpoint as opposed to public domain due to presence of disclaimers. 

http://linuxmafia.com/faq/Licensing_and_Law/public-domain.html

You might want to consider that. 

Comment 3 Daniel Drake 2008-08-04 19:22:47 UTC
Thanks for the feedback! I changed it to MIT and uploaded v1.2.

We already have the appropriate package-specific depends: PolicyKit
We additionally depend on the directory because PolicyKit upstream told us the location where these files are stored may change in future. This dependency will make sure that we notice if/when this change happens.

Comment 4 Rahul Sundaram 2008-08-07 20:48:07 UTC
Sorry for the delay. Approved. Please proceed for cvs admin request. 

https://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

Comment 5 Daniel Drake 2008-08-07 22:25:26 UTC
New Package CVS Request
=======================
Package Name: PolicyKit-olpc
Short Description: OLPC-specific PolicyKit overrides
Owners: dsd
Branches: F-9
InitialCC: dsd
Cvsextras Commits: yes

Comment 6 Kevin Fenzi 2008-08-08 04:14:45 UTC
cvs done.

Comment 7 Fedora Update System 2008-08-08 16:06:05 UTC
PolicyKit-olpc-1.2-1.fc9 has been submitted as an update for Fedora 9

Comment 8 Daniel Drake 2008-08-12 14:51:44 UTC
Thanks for reviewing, we're all done here.

Comment 9 Fedora Update System 2008-08-12 18:19:33 UTC
PolicyKit-olpc-1.2-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 10 Christoph Wickert 2009-07-18 13:35:15 UTC
(In reply to comment #2)
> I can't verify functionality but it seems better to add a package dependency
> instead of the file dependency on /var/lib/PolicyKit-public.

IMO this was a really bad idea, because yum runs out of memory on the XO when resolving file based deps.

Comment 11 Rahul Sundaram 2009-07-18 13:53:35 UTC
Does it make sense to add comments to a review done more than six months back? You seem to actually agree with what I said. Don't quite see the point.

Comment 12 Christoph Wickert 2009-07-18 16:32:14 UTC
I think it does, because I'm referring to a problem you already raised in the review. IMO you should have insisted on following the guidelines here, because they clearly state: "Whenever possible you should avoid file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin."
See http://fedoraproject.org/wiki/Packaging/Guidelines#FileDeps

Comment 13 Rahul Sundaram 2009-07-18 16:47:26 UTC
Read what I have written very carefully one more time. I am advocating against file based dependencies. What exactly is your beef? Drop the witch hunt digging up old reviews.

Comment 14 Christoph Wickert 2009-07-18 17:24:49 UTC
(In reply to comment #13)
> Read what I have written very carefully one more time. I am advocating against
> file based dependencies.

I *did* read what you wrote and I fully understand we both agree on the topic. However I don't understand why you approved the package with the file based dep.

> What exactly is your beef? Drop the witch hunt digging up old reviews.

I want to enhance the review quality in Fedora and IMHO these three word reviews ("Looks good, approved") should be forbidden.

Comment 15 Rahul Sundaram 2009-07-18 19:00:58 UTC
The idea of looking into six month old reviews of only mine because of your desire to improve quality does not sound very convincing. 

You might not know this but Daniel Drake works for OLPC. If there are any performance issues with the choice of a file based dependency, he would fixed it by now. There are no need force a common sense recommendation in the guidelines into a mandatory check point and file based dependencies are NOT forbidden at all. All the guidelines have been followed and the review quality is fine. If you disagree and want to dig up old reviews, go ahead and take it up to FESCo. Until that point, I have nothing further to say on this topic.

Comment 16 Christoph Wickert 2009-07-19 00:20:10 UTC
(In reply to comment #15)
> The idea of looking into six month old reviews of only mine because of your
> desire to improve quality does not sound very convincing.

Please don't take this personally, I'm not picking on you. I'm definitely not only doing this with your reviews, but also with others whenever I stumble upon something that IMO should have been fixed in the review (e. g. bug 502920).

> You might not know this but Daniel Drake works for OLPC. 

Rahul, I *can* read and I do know Daniel from various OLPC-related IRC conversations.

> All the guidelines have been followed and the review quality
> is fine.

It should be obvious for others what you have checked and what not.

Comment 17 Rahul Sundaram 2009-07-19 08:39:00 UTC
Yes, it should be obvious and until the point you find a actual problem, there is nothing to discuss.