Bug 498846
| Summary: | Review Request: R-RM2 - Revenue Management and Pricing for R | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Denis Arnaud <denis.arnaud_fedora> | ||||||||
| Component: | Package Review | Assignee: | Jason Tibbitts <j> | ||||||||
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||
| Severity: | medium | Docs Contact: | |||||||||
| Priority: | medium | ||||||||||
| Version: | rawhide | CC: | bugs.michael, fedora-package-review, notting, susi.lehtola | ||||||||
| Target Milestone: | --- | Flags: | j:
fedora-review+
kevin: fedora-cvs+ |
||||||||
| Target Release: | --- | ||||||||||
| Hardware: | All | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | |||||||||||
| Fixed In Version: | 0.0-4.fc11 | Doc Type: | Bug Fix | ||||||||
| Doc Text: | Story Points: | --- | |||||||||
| Clone Of: | Environment: | ||||||||||
| Last Closed: | 2009-07-16 07:14:37 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: | 498845 | ||||||||||
| Bug Blocks: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Denis Arnaud
2009-05-03 23:14:52 UTC
This fails to build for me in the %install section: * Installing *source* package 'RM2' ... ** R ** preparing package for lazy loading Error in loadNamespace(i[[1L]], c(lib.loc, .libPaths())) : there is no package called 'mvtnorm' Error : package 'msm' could not be loaded ERROR: lazy loading failed for package 'RM2' Does this need a build dependency on R-mvtnorm? Created attachment 349240 [details]
R-mvtnorm spec file
File attach bit me -- this should preceed my attachment
Many other packages need R-mvtnorm as well:
R-mvtnorm is needed by (installed) R-pscl-1.03-1orc.01.x86_64
R-mvtnorm is needed by (installed) R-sampleSelection-0.6.4-1orc.noarch
R-mvtnorm is needed by (installed) R-vcd-1.2.4-1orc.01.noarch
R-mvtnorm is needed by (installed) R-colorspace-1.0.1-1orc.10.x86_64
There is also a needed R and BR dependency on: R-msm to pass %check
The R2spec tool, in Fedora, produces a much more useful URL, and a nice set of pointers to BR's and R's --- I attach .spec's for R-msm, R-mvtnorm, and a revised one for RM2 made using it for comparison, which fedora may prefer to use
-- Russ herrold
Created attachment 349243 [details]
R-RM2 spec
Created attachment 349244 [details]
R-msm spec
Thanks for the update. I'll have a look at it this week-end. That package (R-RM2) depends on R-msm, which is now available in Fedora (but was not at the time I started with R-RM2). Now that I gained experience on packaging R-msm, I'll retrofit it (as well as your feedback) into the R-RM2 packaging. The dependency on R-msm (and R-mvtnorm) has been taken into account. The new versions are accessible at the following URLs: ----------------------------------------------------- Spec URL: http://denisarnaud.fedorapeople.org/R/RM2/2/R-RM2.spec SRPM URL: http://denisarnaud.fedorapeople.org/R/RM2/2/R-RM2-0.0-2.fc11.src.rpm ----------------------------------------------------- rpmlint just throws the classical warning (https://fedoraproject.org/wiki/Packaging/R#Generating_the_search_index.txt). So, the package seems fine for me. Indeed, this builds fine now and rpmlint has only the usual two complaints.
Please don't define a macro (%packrel) for your release; this makes it difficult for others (or the mass rebuild scripts) who may do maintenance on the package. Just use
Release: 2%{?dist}
The license for this package seems to be GPLv3+, not GPLv3. Or did I miss somewhere where later versions of the GPL or not permitted?
* source files match upstream. sha256sum:
21c528b1431d0ef1ea9ab9486a61e58de184172e9a4451b532658b719beb218b
RM2_0.0.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
X license field does not match the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
R-RM2 = 0.0-2.fc12
=
/bin/sh
R
R-msm
R-mvtnorm
* %check is present and all tests pass.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files.
* scriptlets are OK (R package registration).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
The package review process needs reviewers! If you haven't done any package
reviews recently, please consider doing one.
Thanks for that review!
1. You are absolutely right about the license. The authors specify "GPL >= 3"; so, GPLv3+ must be used (instead of GPLv3).
2. As for the %{packrel} macro, as I now understand, it should be used for the release of the upstream R package (and not the RPM release).
The updated specification file and source RPM are to be found at the following URLs:
-----------------------------------------------------
Spec URL: http://denisarnaud.fedorapeople.org/R/RM2/3/R-RM2.spec
SRPM URL: http://denisarnaud.fedorapeople.org/R/RM2/3/R-RM2-0.0-3.fc11.src.rpm
-----------------------------------------------------
As for the package review process, besides a few reviews, I watch almost all the C++-related packages, so as to try to help where there is no other reviewer assigned (it is not that much, but I own quite a few upstream open source projects, which I have to push if I ever want them to reach a critical mass). However, you are right, and I do fully understand that we are never enough to perform peer reviews :)
Looks good; the license is fixed and Release: isn't obfuscated. In general I'd suggest removing commented lines from the %files section and simply not defining unused macros like %packrel, but these are hardly blockers. APPROVED New Package CVS Request ======================= Package Name: R-RM2 Short Description: Revenue Management and Pricing for R Owners: denisarnaud Branches: EL-4 EL-5 F-10 F-11 InitialCC: Taking into account comment #10, the updated specification file and source RPM are to be found at the following URLs: ----------------------------------------------------- Spec URL: http://denisarnaud.fedorapeople.org/R/RM2/4/R-RM2.spec SRPM URL: http://denisarnaud.fedorapeople.org/R/RM2/4/R-RM2-0.0-4.fc11.src.rpm ----------------------------------------------------- If nobody vetoes, that is the version I will import in CVS. CVS done. R-RM2-0.0-4.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/R-RM2-0.0-4.fc11 R-RM2-0.0-4.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. Package Change Request ====================== Package Name: R-RM2 New Branches: F-12 Owners: denisarnaud cvs done. |