Bug 591032

Summary: Review Request: R-xtable - Export tables to LaTeX or HTML
Product: [Fedora] Fedora Reporter: Sandro Mathys <sandro>
Component: Package ReviewAssignee: Pierre-YvesChibon <pingou>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, pingou
Target Milestone: ---Flags: pingou: fedora‑review+
kevin: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: R-xtable-1.5.6-2.fc12 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-05-13 15:23:46 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Sandro Mathys 2010-05-11 05:34:28 EDT
Spec URL: http://red.fedorapeople.org/SRPMS/R-xtable.spec
SRPM URL: http://red.fedorapeople.org/SRPMS/R-xtable-1.5.6-1.fc12.src.rpm
Description:
Coerce data to LaTeX and HTML tables.

This is my very first R package, so I'd welcome if the reviewer had some R knowledge and would double check everything's as it's supposed to be. I used R2spec to start the packaging which seems to guarantee a certain level of awesomeness without much brain energy, tho.

rpmlint {SPECS,SRPMS,RPMS/noarch}/R-xtable*
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
Comment 1 Pierre-YvesChibon 2010-05-11 05:54:05 EDT
Quickly going through the spec:
BuildRequires:    tetex-latex
needs to be
BuildRequires:    tex(latex)

Since you have a noarch package, you will need to add 
Requires:         R-core

You might want to change the Requires to R-core, you don't really need R-devel ;)

You might want to adapt the source0 (up to you) and to change:
%{_datadir}/R/library/%{packname}/data/tli.txt
to
%{_datadir}/R/library/%{packname}/data/

These are esthetic changes mainly, I'll do the proper review later.
Comment 2 Sandro Mathys 2010-05-11 08:36:22 EDT
Spec URL: http://red.fedorapeople.org/SRPMS/R-xtable.spec
SRPM URL: http://red.fedorapeople.org/SRPMS/R-xtable-1.5.6-2.fc12.src.rpm

Thanks for the initial review. Made the proposed changes and some other stuff I noticed while working on other R packages and talking to you on IRC :) Also added BR: texinfo-tex as it's needed in %check

rpmlint output is still clean :)
Comment 3 Pierre-YvesChibon 2010-05-12 03:50:47 EDT
* source files match upstream:
   sha1sum xtable_1.5-6.tar.gz 
2eb74a6d55c0b38d07180f876ec63ce8fbd77742  xtable_1.5-6.tar.gz
   sha1sum rpmbuild/SOURCES/xtable_1.5-6.tar.gz 
2eb74a6d55c0b38d07180f876ec63ce8fbd77742  rpmbuild/SOURCES/xtable_1.5-6.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.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
! BuildRequires are proper.
  You should BR on R-devel rather than R

* %clean is present.
* package builds in koji (
http://koji.fedoraproject.org/koji/taskinfo?taskID=2182035 ).
* package installs properly
* rpmlint produces 4 warnings, safe to ignore (spelling).
  2 packages and 0 specfiles checked; 0 errors, 0 warnings.
* final provides and requires are sane
* %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.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

I trust you can correct the BR before uploading and building in the CVS.

** APPROVED **
Comment 4 Sandro Mathys 2010-05-12 04:10:20 EDT
Right, just changed that BR locally. Thanks for the review Pierre-Yves!

New Package CVS Request
=======================
Package Name: R-xtable
Short Description: Export tables to LaTeX or HTML
Owners: red
Branches: F-12 F13
InitialCC:
Comment 5 Kevin Fenzi 2010-05-12 13:24:37 EDT
CVS done (by process-cvs-requests.py).
Comment 6 Fedora Update System 2010-05-13 03:49:45 EDT
R-xtable-1.5.6-2.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/R-xtable-1.5.6-2.fc12
Comment 7 Fedora Update System 2010-05-13 03:50:31 EDT
R-xtable-1.5.6-2.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/R-xtable-1.5.6-2.fc13
Comment 8 Fedora Update System 2010-05-13 15:23:42 EDT
R-xtable-1.5.6-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 9 Fedora Update System 2010-05-13 15:31:20 EDT
R-xtable-1.5.6-2.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.