Bug 1126459
Summary: | Review Request: qqwing - library for sudoku generation and solving | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Michael Catanzaro <mcatanzaro+wrong-account-do-not-cc> |
Component: | Package Review | Assignee: | Kalev Lember <kalevlember> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | kalevlember, package-review |
Target Milestone: | --- | Flags: | kalevlember:
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: | 2014-08-19 22:32:38 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
Michael Catanzaro
2014-08-04 13:25:45 UTC
Whoops, that SPRM did not match that spec. Try #2: https://people.gnome.org/~mcatanzaro/qqwing2/libqqwing.spec https://people.gnome.org/~mcatanzaro/qqwing2/libqqwing-1.1.2-1.fc20.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=7239610 rpmlint says: libqqwing.x86_64: E: incorrect-fsf-address /usr/share/doc/libqqwing/COPYING libqqwing-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/qqwing-1.1.2/qqwing.cpp libqqwing-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/qqwing-1.1.2/qqwing.hpp libqqwing-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/qqwing-1.1.2/main.cpp libqqwing-devel.x86_64: W: only-non-binary-in-usr-lib libqqwing-devel.x86_64: W: no-documentation libqqwing-devel.x86_64: E: incorrect-fsf-address /usr/include/qqwing.hpp qqwing.x86_64: W: no-manual-page-for-binary qqwing 5 packages and 1 specfiles checked; 5 errors, 3 warnings. I've added a man page and fixed the incorrect-fsf-address errors upstream. only-non-binary-in-usr-lib is complaining about the pkgconfig file. The devel package indeed has no documentation. Blocking FE-NEEDSPONSOR since I'm not a packager yet. Kalev has agreed to sponsor me. Hi Michael, Thanks for packaging this up and sorry for the delay; I'm now back from GUADEC / Flock so lets try and get this in. The spec file looks nice and clean, but I have a question about naming. Usually packages in Fedora follow the convention where upstream tarball name == srpm name. This often makes packaging simpler and easier to understand. Have you considered calling the spec/srpm "qqwing" and name the binary packages "qqwing", "qqwing-devel" and "qqwing-libs"? It's definitely not a blocker and it's fine the way it is now too, just wondering if you had any special reason for doing a slightly different thing here. Since qqwing (the executable) depends on libqqwing, I thought it might be odd that the main package depended on the subpackage; with libqqwing as the main package, the subpackage depends on the main package. That was my only reason. If it's OK for the main package to require the subpackage, then I'll change it so that qqwing is the base package and libqqwing the subpackage. Having the main package depend on a subpackage is very common. Nothing wrong with doing that. Just a random example that does this: http://pkgs.fedoraproject.org/cgit/evince.git/tree/evince.spec OK, updated to make libqqwing the subpackage, and also use the latest upstream release: https://people.gnome.org/~mcatanzaro/qqwing3/qqwing.spec https://people.gnome.org/~mcatanzaro/qqwing3/qqwing-1.1.3-1.fc20.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=7310772 libqqwing-devel.x86_64: W: only-non-binary-in-usr-lib libqqwing-devel.x86_64: W: no-documentation 5 packages and 1 specfiles checked; 0 errors, 2 warnings. One more try, using the idiomatic package names you suggested this time: https://people.gnome.org/~mcatanzaro/qqwing4/qqwing.spec https://people.gnome.org/~mcatanzaro/qqwing4/qqwing-1.1.3-1.fc20.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=7310808 qqwing-devel.x86_64: W: spelling-error Summary(en_US) libqqwing -> liberating qqwing-devel.x86_64: W: spelling-error %description -l en_US libqqwing -> liberating qqwing-devel.x86_64: W: only-non-binary-in-usr-lib qqwing-devel.x86_64: W: no-documentation qqwing-libs.x86_64: W: spelling-error %description -l en_US libqqwing -> liberating 5 packages and 1 specfiles checked; 0 errors, 5 warnings. This looks great to me, thanks! Two very minor nits: > %description devel > The libqqwing-devel package contains libraries and header files for > developing applications that use libqqwing. The description needs updating after the libqqwing -> qqwing rename. Could use the %{name} macro here since it semantically refers to the base package's name. > %{_mandir}/man1/qqwing.1.gz Would be better to use a glob here to make sure this line works when the man page compression used by rpmbuild gets changed. Something like: %{_mandir}/man1/qqwing.1* Fedora review qqwing-1.1.3-1.fc20.src.rpm 2014-08-18 + OK ! needs attention + rpmlint warnings are harmless and can be ignored + The package is named according to Fedora packaging guidelines + The spec file name matches the base package name. + The package meets the Packaging Guidelines + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The license field in the spec file matches the actual license + The package contains the license file (COPYING) + Spec file is written in American English + Spec file is legible + Upstream sources match the sources in the srpm a3822456e01f74db6e7a2e5ae135ad78 qqwing-1.1.3.tar.gz a3822456e01f74db6e7a2e5ae135ad78 Download/qqwing-1.1.3.tar.gz + The package builds in koji n/a ExcludeArch bugs filed + BuildRequires look sane n/a locale handling + ldconfig in %post and %postun + Package does not bundle copies of system libraries n/a Package isn't relocatable + Package owns all the directories it creates + No duplicate files in %files + Permissions are properly set + Consistent use of macros + The package must contain code or permissible content n/a Large documentation files should go in -doc subpackage + Files marked %doc should not affect package + Header files should be in -devel n/a Static libraries should be in -static + Library files that end in .so must go in a -devel package + -devel must require the fully versioned base + Packages should not contain libtool .la files n/a Proper .desktop file handling + Doesn't own files or directories already owned by other packages + Filenames are valid UTF-8 APPROVED The package looks good and feel free to fix up the minor issues I pointed out in comment #8 when commiting this to git; no need to upload the spec / srpm files for another review round. Or you can, of course, if you want to :-) Next thing would be sponsoring you to the packager group so you can request the git repo creation. One thing that I usually ask new packagers to do before sponsoring them is to go through some other review request and try to do an official review there. Every packager is also automatically a reviewer and it's important to know how to review other people's packages. For a review, I usually just go through the review checklist in https://fedoraproject.org/wiki/Packaging:ReviewGuidelines and post a short comment for each MUST line, indicating whether it passes or not. There's a helper called fedora-review to automate this, but it can be instructional to go through this manually first. Anyway, I don't want to delay this package any more, since it's the 3.13.90 tarball date today. Would be great if you could look into reviewing something (maybe GNOME-related?) one day. So, welcome to Fedora packaging Michael! Glad to have you. I've sponsored you now to the packager group; the permissions might take up to an hour to sync everywhere. Would be good if you could join the announce lists (announce and devel-announce). Feel free to look me up anytime, either on IRC or by email. Happy to help find answers to any packaging related questions. New Package SCM Request ======================= Package Name: qqwing Short Description: Sudoku solver and generator Upstream URL: http://ostermiller.org/qqwing/ Owners: Catanzaro Branches: f21 f22 InitialCC: WARNING: "Catanzaro" is not a valid FAS account. WARNING: Invalid branch f22 requested f22 is master/devel and is automatic. Thanks Jon, sorry about that! New Package SCM Request ======================= Package Name: qqwing Short Description: Sudoku solver and generator Upstream URL: http://ostermiller.org/qqwing/ Owners: catanzaro Branches: f21 InitialCC: Git done (by process-git-requests). OK, qqwing is built for F21 and F22, so you should be good to go on GNOME Sudoku. |