Bug 1263821
Summary: | Review Request: dput-ng - Next generation Debian package upload tool | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Michael Kuhn <suraia> |
Component: | Package Review | Assignee: | Hans de Goede <hdegoede> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | hdegoede, package-review |
Target Milestone: | --- | Flags: | hdegoede:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2016-01-25 02:21:43 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: | |||
Bug Depends On: | 1278964 | ||
Bug Blocks: |
Description
Michael Kuhn
2015-09-16 19:33:41 UTC
I have updated the package to add a dependency on distro-info, which provides additional functionality (package review #1278964). The SRPM URL has changed due to a Fedora upgrade; the new URLs are as follows: Spec URL: https://ikkoku.de/~suraia/dput-ng/dput-ng.spec SRPM URL: https://ikkoku.de/~suraia/dput-ng/dput-ng-1.10-1.fc23.src.rpm Hi, As discussed per email, I will review this and sponsor you once dput-ng and all its dependencies have passed their pkg review. Regards, Hans Full review done: Good: ==== - package meets naming guidelines - package meets packaging guidelines - license (GPLv2+ and MIT) OK, matches source - spec file legible, in am. english - source matches upstream - package compiles on devel (x86) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file Needs work: ======== - rpmlint checks return: dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/profiles/security-master.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/metas/boring.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/profiles/DEFAULT.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/profiles/mentors.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/profiles/ppa.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/profiles/ssh-upload.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/profiles/ubuntu.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/metas/ubuntu.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/profiles/ftp-master.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/metas/debomatic.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/profiles/ftp-eu.json dput-ng.noarch: W: conffile-without-noreplace-flag /etc/dput.d/metas/debian.json These warnings can be silenced by replacing %config with %config(noreplace), the question i, is this the right thing todo or should we just ignore the warnings? %config(noreplace) means that if the user modifies these files that any (modified) files in the rpm will get installed as etc/dput.d/foo/bar.json.rpmnew, and the user version will be kept. Ideally the system version of these files would live under /usr/share/dput.d/foo/bar.json, and dput would first check for a user version under /etc/dput.d/ and if that not exists fallback to the system defaults from /usr/share/dput.d I'm fine with keeping this as is, adding the (noreplace), or moving the files enitrely to /usr/share (if user modification is not expected / not desirable). Your call. - license text not marked as %license, the LICENSE file should be a seperate line in %files starting with %license, rather then being part of %doc If you can create a 2 srpm fixing these 2 issues (note as said for the first issue, not taking any action is an acceptable solution), then we should be good to go wrt this pkg. > - rpmlint checks return: > dput-ng.noarch: W: conffile-without-noreplace-flag > /etc/dput.d/profiles/security-master.json > … > I'm fine with keeping this as is, adding the (noreplace), or moving the > files enitrely to /usr/share (if user modification is not expected / not > desirable). Your call. Good point, I simply used this layout because that is how the Debian package does it. I have now checked the documentation again and dput-ng first checks /usr/share, then /etc and then ~. I have now moved all files to /usr/share as they can be easily overridden using appropriate files in /etc or ~ (but this is usually not necessary). > - license text not marked as %license, the LICENSE file should be a seperate > line in %files starting with %license, rather then being part of %doc Done. By the way, I guess the Fedora wiki page (https://fedoraproject.org/wiki/How_to_create_an_RPM_package) is out-of-date then because it still contains the following sentence: “These prefixes are not valid in Fedora: %license and %readme.” Should I just update this? > If you can create a 2 srpm fixing these 2 issues (note as said for the first > issue, not taking any action is an acceptable solution), then we should be > good to go wrt this pkg. Spec: https://ikkoku.de/~suraia/dput-ng/dput-ng.spec SRPM: https://ikkoku.de/~suraia/dput-ng/dput-ng-1.10-2.fc23.src.rpm (In reply to Michael Kuhn from comment #4) > > By the way, I guess the Fedora wiki page > (https://fedoraproject.org/wiki/How_to_create_an_RPM_package) is out-of-date > then because it still contains the following sentence: “These prefixes are > not valid in Fedora: %license and %readme.” > Should I just update this? Yes please. I will look at the new versions of all 3 pkgs you've submitted tomorrow. Based on the comment at https://bugzilla.redhat.com/show_bug.cgi?id=1278964#c5, I have updated the package according to the new Python packaging guidelines. Spec: https://ikkoku.de/~suraia/dput-ng/dput-ng.spec SRPM: https://ikkoku.de/~suraia/dput-ng/dput-ng-1.10-3.fc23.src.rpm (In reply to Michael Kuhn from comment #6) > Based on the comment at > https://bugzilla.redhat.com/show_bug.cgi?id=1278964#c5, I have updated the > package according to the new Python packaging guidelines. > > Spec: https://ikkoku.de/~suraia/dput-ng/dput-ng.spec > SRPM: https://ikkoku.de/~suraia/dput-ng/dput-ng-1.10-3.fc23.src.rpm Looks good to me now: Approved. I've added you to the packagers group and sponsored you, so now you can continue with the next steps: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner Regards, Hans Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/dput-ng dput-ng-1.10-3.fc23 distro-info-0.14-3.fc23 distro-info-data-0.28-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-327c80950a distro-info-0.14-3.fc23, distro-info-data-0.28-3.fc23, dput-ng-1.10-3.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-327c80950a distro-info-0.14-3.fc23, distro-info-data-0.28-3.fc23, dput-ng-1.10-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report. |