Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: travelccm - C++ Travel Customer Choice Model Library
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Summary: Review Request: travelccm - C++ Travel Customer Choice Model Library Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: denis.arnaud_fedora@m4x.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
Spec URL: http://denisarnaud.fedorapeople.org/sim/travelccm/travelccm-0.5.0-1.spec SRPM URL: http://denisarnaud.fedorapeople.org/sim/travelccm/travelccm-0.5.0-1.fc15.src... Description: TravelCCM aims at providing a clean API, and the corresponding C++ implementation, for choosing one item among a set of travel solutions, given demand-related characteristics (e.g., Willingness-To-Pay, preferred airline, preferred cabin, etc.).
The TravelCCM C++ library implements some simple Customer Choice Models (CCM), as referenced in the literature (PhD dissertations at MIT, for instance: http://dspace.mit.edu).
The TravelCCM C++ library exposes a simple, clean and object-oriented, API. For instance, the choose() method takes, as input, both a structure representing the travel request (e.g., "from Washington, DC, US, to Beijing, China, on the 25th of May") and a list of travel solutions (as provided by the Airline Schedule Manager project: http://sourceforge.net/projects/air-sched), and yields, as output, the chosen item.
The output can then be used by other systems, for instance to book the corresponding travel or to visualize it on a map and calendar and to share it with others.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Thomas Spura tomspur@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |tomspur@fedoraproject.org AssignedTo|nobody@fedoraproject.org |tomspur@fedoraproject.org Flag| |fedora-review+
--- Comment #1 from Thomas Spura tomspur@fedoraproject.org 2011-09-23 10:56:24 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
--- Comment #2 from Denis Arnaud denis.arnaud_fedora@m4x.org 2011-09-23 13:55:56 EDT --- 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?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
--- Comment #3 from Thomas Spura tomspur@fedoraproject.org 2011-09-23 14:56:48 EDT --- (In reply to comment #2)
Thanks for the review!
- 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.
- 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 :(
- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Denis Arnaud denis.arnaud_fedora@m4x.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW CC| |denis.arnaud_fedora@m4x.org Depends on| |702987
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
--- Comment #4 from Denis Arnaud denis.arnaud_fedora@m4x.org 2011-09-24 13:29:18 EDT --- 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... ===================================================
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/travel... It could give ideas to some other package maintainers :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Thomas Spura tomspur@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
--- Comment #5 from Thomas Spura tomspur@fedoraproject.org 2011-10-01 07:27:25 EDT --- (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/travel... 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Denis Arnaud denis.arnaud_fedora@m4x.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #6 from Denis Arnaud denis.arnaud_fedora@m4x.org 2011-10-01 14:30:14 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
--- Comment #7 from Jon Ciesla limb@jcomserv.net 2011-10-03 08:22:05 EDT --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
--- Comment #8 from Fedora Update System updates@fedoraproject.org 2011-10-04 09:15:41 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
--- Comment #9 from Fedora Update System updates@fedoraproject.org 2011-10-04 09:16:29 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
--- Comment #10 from Fedora Update System updates@fedoraproject.org 2011-10-04 09:49:50 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #11 from Fedora Update System updates@fedoraproject.org 2011-10-04 16:47:40 EDT --- travelccm-0.5.0-2.fc16 has been pushed to the Fedora 16 testing repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |travelccm-0.5.0-2.fc15 Resolution| |ERRATA Last Closed| |2011-10-12 20:47:02
--- Comment #12 from Fedora Update System updates@fedoraproject.org 2011-10-12 20:47:02 EDT --- travelccm-0.5.0-2.fc15 has been pushed to the Fedora 15 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|travelccm-0.5.0-2.fc15 |travelccm-0.5.0-2.fc14
--- Comment #13 from Fedora Update System updates@fedoraproject.org 2011-10-12 20:50:52 EDT --- travelccm-0.5.0-2.fc14 has been pushed to the Fedora 14 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|travelccm-0.5.0-2.fc14 |travelccm-0.5.0-2.fc16
--- Comment #14 from Fedora Update System updates@fedoraproject.org 2011-10-13 00:38:45 EDT --- travelccm-0.5.0-2.fc16 has been pushed to the Fedora 16 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=732218
Denis Arnaud denis.arnaud_fedora@m4x.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |890772
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=890772 [Bug 890772] Review Request: tvlsim - Travel Market Simulator
package-review@lists.fedoraproject.org