Bug 732218
Summary: | Review Request: travelccm - C++ Travel Customer Choice Model Library | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Denis Arnaud <denis.arnaud_fedora> |
Component: | Package Review | Assignee: | Thomas Spura <tomspur> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | denis.arnaud_fedora, notting, package-review, tomspur |
Target Milestone: | --- | Flags: | tomspur:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | travelccm-0.5.0-2.fc16 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2011-10-13 00:47:02 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: | 702987 | ||
Bug Blocks: | 890772 |
Description
Denis Arnaud
2011-08-20 23:32:21 UTC
Review: Good: - name ok - rpmlint ok (except spec naming below) - BR ok - group ok - R ok - parallel make is used - check is there - ldconfig called - libs correctly installed - no static libs - no *.la - %files ok - sources match upstream (sha512): 20894cc43b0fb5902e30721c10cd3a1c38a1f6b5503e486a92ba8dfb9ffd9a01e25da5a64c56835da6cb47b22ee6d09f921200d58c335c619ca4841b1b6d86ab travelccm-0.5.0.tar.bz2 MUST: - spec has to be travelccm.spec without the version, but I'm sure, you'll change that on importing. See also rpmlint: $ rpmlint ~/rpmbuild/SRPMS/travelccm-0.5.0-1.fc15.src.rpm \ ~/rpmbuild/RPMS/x86_64/travelccm-* \ ~/rpmbuild/RPMS/noarch/travelccm-doc-0.5.0-1.fc15.noarch.rpm travelccm.src: E: invalid-spec-name 5 packages and 0 specfiles checked; 1 errors, 0 warnings. SHOULD: - can the permissions be fixed upstream? ;) - I still think this would be better: %if ! (0%{?rhel} > 5) BuildArch: noarch %endif So the doc package will be noarch on fedora and rhel > 5. Please reconsider, but only SHOULD here... - please add proper LGPLv2+ headers Question: - Why are you running ctest, but suggest to run "make check" in README for checking? ;) _____________________________________________________________________ APPROVED, when you fix spec renaming on importing Thanks for the review! 1. Yes, of course, I will change the specification file name to travelccm.spec. As a matter of fact, locally, to test the build of the package, I usually do a symbolic link on the latest version of the specification file (e.g., travelccm.spec -> travelccm.spec-0.5.0-1.spec). That avoids confusion around the version of the specification file. 2. The permissions can be fixed upstream, of course. Indeed, they are already fixed (I have to check to be 100% sure, but normally, they are). I will remove that part from the specification file (as it has become useless). 3. For the doc sub-package, I guess that you meant: %if ! (0%{?rhel} < 6) BuildArch: noarch %endif It makes sense to me, and I will alter the specification file. 4. About the LGPLv2+ headers, you mean that I should add them into the source code? (In reply to comment #2) > Thanks for the review! > > 1. Yes, of course, I will change the specification file name to travelccm.spec. > As a matter of fact, locally, to test the build of the package, I usually do a > symbolic link on the latest version of the specification file (e.g., > travelccm.spec -> travelccm.spec-0.5.0-1.spec). That avoids confusion around > the version of the specification file. Yes, it will only always cause a rpmlint warning, when you link to the versioned spec. Maybe rsyncing with "-L" would help here. > 3. For the doc sub-package, I guess that you meant: > %if ! (0%{?rhel} < 6) > BuildArch: noarch > %endif > It makes sense to me, and I will alter the specification file. Yes, of course :( > > 4. About the LGPLv2+ headers, you mean that I should add them into the source > code? Hmm, I ought to remember, that's a SHOULD to ask upstream for doing so, e.g.: http://ball-trac.bioinf.uni-sb.de/ticket/220 Feedback has been taken into account: =================================================== Spec URL: http://denisarnaud.fedorapeople.org/sim/travelccm/travelccm-0.5.0-2.spec SRPM URL: http://denisarnaud.fedorapeople.org/sim/travelccm/travelccm-0.5.0-2.fc15.src.rpm =================================================== Note that: -------------- %if ! (0%{?rhel} < 6) BuildArch: noarch %endif does not work on Fedora because %{rhel} is not defined (and therefore the whole condition holds false). Hence, I have used: %if 0%{?fedora} || 0%{?rhel} > 5 BuildArch: noarch %endif -------------- Linking travelccm.spec to travelccm-0.5.0-2.spec, in order to build (with rpmbuild -bs/-ba) and to test (with rpmlint) RPM packaging, fully works: rpmlint does not report any warning. Indeed, it allows me tracking all the different versions for all the review requests and bugs, in a very convenient way: https://github.com/denisarnaud/fedorareviews/tree/trunk/sim/travelccm/travelccm_732218 It could give ideas to some other package maintainers :) (In reply to comment #4) > Feedback has been taken into account: Looks good now. > Hence, I have used: > %if 0%{?fedora} || 0%{?rhel} > 5 > BuildArch: noarch > %endif Yes, fine now. > Indeed, it allows me tracking all the different versions for all the review > requests and bugs, in a very convenient way: > https://github.com/denisarnaud/fedorareviews/tree/trunk/sim/travelccm/travelccm_732218 > It could give ideas to some other package maintainers :) Hmm, when you use git, I'd only modify travelccm.spec and let git do its job with tracking the different versions :) In your case you should upload the travelccm.spec link to fedorapeople, so a reviewer will get that first (and see the history when manually going there and looking at the versioned specs). Feel free to request a new package. New Package SCM Request ======================= Package Name: travelccm Short Description: C++ Travel Customer Choice Model (CCM) Library Owners: denisarnaud Branches: f14 f15 f16 el4 el5 el6 InitialCC: denisarnaud Git done (by process-git-requests). travelccm-0.5.0-2.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/travelccm-0.5.0-2.fc16 travelccm-0.5.0-2.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/travelccm-0.5.0-2.fc15 travelccm-0.5.0-2.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/travelccm-0.5.0-2.fc14 travelccm-0.5.0-2.fc16 has been pushed to the Fedora 16 testing repository. travelccm-0.5.0-2.fc15 has been pushed to the Fedora 15 stable repository. travelccm-0.5.0-2.fc14 has been pushed to the Fedora 14 stable repository. travelccm-0.5.0-2.fc16 has been pushed to the Fedora 16 stable repository. |