Bug 837313
Summary: | Review Request: gssproxy - A proxy for GSSAPI credential handling | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Guenther Deschner <gdeschner> |
Component: | Package Review | Assignee: | Guenther Deschner <gdeschner> |
Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | a.badger, asn, package-review, pingou, ssorce |
Target Milestone: | --- | Flags: | asn:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2015-02-18 14:55:00 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
Guenther Deschner
2012-07-03 12:39:49 UTC
I've reviewed the spec and the source RPM. All good :) New Package SCM Request ======================= Package Name: gssproxy Short Description: A proxy for GSSAPI credential handling Owners: gd simo Branches: f18 devel InitialCC: Andreas -- Could you go back and please do a proper Fedora Review? Just looking at the spec file and not attempting to build or understand gssproxy at all, I immediately see two things wrong with it (Not a complete URL to the Source, scriptlets for installing systemd units are wrong). The lack of any Requires: is also fishy to me... I'm thinking there's probably at least a missing Requires: on some systemd package. The ldconfig invocation in the %post script is also fishy -- I don't see any libraries being installed and if there were libraries installed, there's no matching ldconfig call in %postun. If you dig into the package you may find even more stuff wrong. For help with doing a proper review, you can look at: http://fedoraproject.org/wiki/Packaging:ReviewGuidelines and http://fedoraproject.org/wiki/Packaging:Guidelines The scriptlets for systemd are on: http://fedoraproject.org/wiki/Packaging:Systemd Actually, that was the general systemd-in-packaging page. The scriptlets themselves are on: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd Ok, added various fixes for systemd/rpmlint issues, etc. Also uses first real upstream tarball 0.0.2 now. Please re-review. Spec URL: http://fedorapeople.org/~gd/gssproxy/gssproxy.spec SRPM URL: http://fedorapeople.org/~gd/gssproxy/gssproxy-0.0.2-3.fc17.src.rpm The fixes look fine for me and it builds and installs just fine too. Git done (by process-git-requests). No need to request f18, currently ==devel. This package still doesn't look like it has been properly reviewed. Andreas, there's many templates out there of what needs to be checked on review that you could use (for instance: https://fedoraproject.org/wiki/User:Tibbs/Review_Template ). You can also just use http://fedoraproject.org/wiki/Packaging:ReviewGuidelines and post that in checklist form. If you aren't checking at least the things mentioned in the Packaging Guidelines, then you aren't doing a proper review. So far, your reviews don't appear to have done that (as you missed easily spotted things after your initial review in Comment #1 and your followup in Comment #6 doesn't say that you then checked the package against the Guidelines, only that you built it and installed it.) If you are doing the full review just not writing it down, also remember that the package review is a means of communication. If you don't write down what has been checked, what was good, and what was bad, you aren't recording information on the package that people may need later when figuring out why or when a package started doing something wrong. Guenther, please don't build this package until Andreas has done a full review -- I see at least one other thing that Andreas hasn't done which should lead to either a change in %files or a comment about why it is the way it is. Sorry, package already built. Toshio, what in particular should I fix (and Andreas review) ? You mentioned an item in the files section. Andreas needs to do the review. If I keep telling what things I find, at some point I've done the review, not Andreas. For you, and the package, that's fine. But for Andreas, the only thing that could be done then is decide he doesn't know how to be a Fedora Contributor and ask FESCo to put him on probation. (Which FESCo might or might not grant -- I really hate to go down that path so I try to get people on the right path before it gets to that state :-) --- gssproxy.spec.orig 2012-07-13 12:46:58.000000000 +0200 +++ gssproxy.spec 2012-07-16 21:33:02.740748184 +0200 @@ -36,7 +36,7 @@ %description -A proxy for GSSAPI credential handling +A proxy for GSSAPI credential handling. %prep %setup -q @@ -67,13 +67,14 @@ %files %defattr(-,root,root,-) +%doc COPYING %{_unitdir}/gssproxy.service %{_sbindir}/gssproxy %dir %{gsspstatedir} %attr(755,root,root) %dir %{pipepath} %attr(755,root,root) %dir %{pubconfpath} -%attr(700,root,root) %dir /%{_sysconfdir}/gssproxy -%attr(0600,root,root) %config(noreplace) /%{_sysconfdir}/gssproxy/gssproxy.conf +%attr(700,root,root) %dir %{_sysconfdir}/gssproxy +%attr(0600,root,root) %config(noreplace) %{_sysconfdir}/gssproxy/gssproxy.conf %{_mandir}/man5/gssproxy.conf.5* %{_mandir}/man8/gssproxy.8* For a full review with every step documented you have to wait till tomorrow. Ok, added these and more fixes to the specfile. gssproxy is in fedora meanwhile. |