[Bug 697836] New: Review Request: mmseq - Haplotype and isoform specific expression estimation for RNA-seq
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: mmseq - Haplotype and isoform specific expression estimation for RNA-seq
https://bugzilla.redhat.com/show_bug.cgi?id=697836
Summary: Review Request: mmseq - Haplotype and isoform specific expression estimation for RNA-seq Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: bloch@verdurin.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora Story Points: ---
Spec URL: http://verdurin.fedorapeople.org/reviews/mmseq/mmseq.spec SRPM URL: http://verdurin.fedorapeople.org/reviews/mmseq/mmseq-0.9.9-2.fc16.src.rpm Description: Software and instructions for fast, scalable isoform expression estimation using multi-mapping RNA-seq reads.
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=697836
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |volker27@gmx.at
--- Comment #1 from Volker Fröhlich volker27@gmx.at 2011-05-06 18:44:39 EDT --- Just some comments:
Please include the license file. The postal addresses of the FSF are outdated in several files.
Use -p on the install commands, at least for the scripts. One of the scripts uses the samtools binary, so you'll need an explicit Require on that, as RPM will only install samtools-libs
Besides the first part of the package description is pretty generic, it says, it would contain instructions. I can't see any and especially the mmseq binary is especially sparse on saying anything. Why does the description not have the word "Haplotype"?
Why did you choose to not install the Perl script?
You might want to add "-b .name_of_the_patch" to the patch macros. Please also comment on the patches.
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=697836
--- Comment #2 from Adam Huffman bloch@verdurin.com 2011-05-08 08:01:13 EDT --- (In reply to comment #1)
Just some comments:
Thanks for looking
Please include the license file. The postal addresses of the FSF are outdated in several files.
File included and I'll contact upstream about the out of data FSF address.
Use -p on the install commands, at least for the scripts. One of the scripts uses the samtools binary, so you'll need an explicit Require on that, as RPM will only install samtools-libs
Done.
Besides the first part of the package description is pretty generic, it says, it would contain instructions. I can't see any and especially the mmseq binary is especially sparse on saying anything. Why does the description not have the word "Haplotype"?
I was trying to differentiate the description from the summary but have improved it now.
Why did you choose to not install the Perl script?
Fixed.
You might want to add "-b .name_of_the_patch" to the patch macros. Please also comment on the patches.
Done.
I've also updated to the latest upstream release.
http://verdurin.fedorapeople.org/reviews/mmseq/mmseq.spec
http://verdurin.fedorapeople.org/reviews/mmseq/mmseq-0.9.10b-1.fc16.src.rpm
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=697836
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |volker27@gmx.at Flag| |fedora-review?
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=697836
--- Comment #3 from Adam Huffman bloch@verdurin.com 2011-05-13 19:41:32 EDT --- The upstream developer has said he'll fix the FSF address problems in the next release.
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=697836
--- Comment #4 from Volker Fröhlich volker27@gmx.at 2011-05-14 05:55:08 EDT --- The naming guidelines are not met for this pre-release version. Please see the review below for details and correct it.
Concerning the current FSF address, please see https://bugzilla.redhat.com/show_bug.cgi?id=690919#c5 It is not considered as a blocker, so it is up to you.
Please correct the VERSION file prior building, as it contains the wrong version number. I'd also exclude it from the file section, as it serves no use as far as I can see.
I'd prefer having a new line for every BuildRequires, as it is clearer. You can also be more specific on the script file names in the files section.
If you don't plan to introduce the package into EPEL, you can drop the clean section, the rm -rf %{buildroot} and the BuildRoot definition. It seems like, there are no samtools in EPEL yet.
Remove the extra white-space in front of the word "Example".
I'm not sure if zlib is used at all. It's in the flags, but I can't see how they'd use it. This might not be important.
-------------------------------------------------------- Review:
[+] Good [-] Needs work [0] Does not apply
MUST: =====
[+] rpmlint:
[makerpm@fedora14 mmseq_0.9.10b]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/mmseq-* mmseq.src: W: spelling-error Summary(en_US) Haplotype -> Haplology, Holotype, Haplography mmseq.src: W: spelling-error Summary(en_US) isoform -> iodoform, isomorph, proforma mmseq.src: W: spelling-error %description -l en_US haplotype -> haplology, holotype, haplography mmseq.src: W: spelling-error %description -l en_US isoform -> iodoform, isomorph, proforma mmseq.src: W: spelling-error %description -l en_US multi -> mulch, mufti mmseq.x86_64: W: spelling-error Summary(en_US) Haplotype -> Haplology, Holotype, Haplography mmseq.x86_64: W: spelling-error Summary(en_US) isoform -> iodoform, isomorph, proforma mmseq.x86_64: W: spelling-error %description -l en_US haplotype -> haplology, holotype, haplography mmseq.x86_64: W: spelling-error %description -l en_US isoform -> iodoform, isomorph, proforma mmseq.x86_64: W: spelling-error %description -l en_US multi -> mulch, mufti mmseq.x86_64: E: incorrect-fsf-address /usr/bin/testregexp.rb mmseq.x86_64: E: incorrect-fsf-address /usr/bin/sam2hits.rb mmseq.x86_64: E: incorrect-fsf-address /usr/bin/filterGTF.rb mmseq.x86_64: E: incorrect-fsf-address /usr/bin/fastagrep.sh mmseq.x86_64: E: incorrect-fsf-address /usr/bin/haploref.rb mmseq.x86_64: W: no-manual-page-for-binary haploref.rb mmseq.x86_64: W: no-manual-page-for-binary ensembl_gtf_to_gff.pl mmseq.x86_64: W: no-manual-page-for-binary mmseq mmseq.x86_64: W: no-manual-page-for-binary get_isize.rb mmseq.x86_64: W: no-manual-page-for-binary bam2hits mmseq.x86_64: W: no-manual-page-for-binary fastagrep.sh mmseq.x86_64: W: no-manual-page-for-binary filterGTF.rb mmseq.x86_64: W: no-manual-page-for-binary sam2hits.rb mmseq.x86_64: W: no-manual-page-for-binary testregexp.rb mmseq.x86_64: W: no-manual-page-for-binary pileup.sh mmseq-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mmseq_0.9.10b/mmseq.cpp mmseq-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mmseq_0.9.10b/sokal.cc 3 packages and 0 specfiles checked; 7 errors, 20 warnings.
[-] Naming according to the Package Naming Guidelines: Please see http://fedoraproject.org/wiki/PackageNamingGuidelines#NonNumericRelease [+] Spec file matches base package name [+] Packaging guidelines met [+] License approved for Fedora [+] License field in spec matches code [+] License file included, if source package includes it [+] Spec in American English [+] Spec is legible [+] Sources match upstream md5sum: 1f1c5b338eec23994fd84edb7e00b17a [+] Compiles and builds into binary RPMs on at least one primary architecture: http://koji.fedoraproject.org/koji/taskinfo?taskID=3071848
[0] ExcludeArch is specified and commented [0] Locales are handled correctly [+] All build dependencies listed [0] Calls ldconfig for its shared libraries [+] No bundled system libraries [0] Stated as relocatable package [+] Owns all its directories or requires package that does [+] No file listing duplicates [+] File permissions correct [+] Consistent use of macros [+] Code or permissible content [0] Large documentation in -doc subpackage [+] No runtime dependency of files listed as %doc [0] Header files in -devel subpackage [0] Static files in -static subpackage [0] Library files without suffix in -devel subpackage [0] Devel-package requires base package [0] No .la libtool archives [0] GUI application includes properly installed %{name}.desktop file [+] No files or directories owned, that other packages own [+] Filenames in packages are UTF-8
SHOULD: =======
[0] Query upstream if no license text is included [+] Package builds in mock: Tried fedora-rawhide-x86_64, fedora-rawhide-i386 [?] Package works as described -- Haven't tried [0] Scriptlets are sane, if used [0] Subpackages other than -devel should require base package (versioned) [0] pkgconfig files in -devel subpackage [0] Dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself [-] Contain man pages, where they make sense -- Please try to query upstream on this.
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=697836
--- Comment #5 from Volker Fröhlich volker27@gmx.at 2011-05-14 09:44:06 EDT --- Replacing "-lz" with "-Wl,--as-needed -lz" proves right: zlib is unnecessary. You can drop from the BRs and remove it from the Makefile.
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=697836
--- Comment #6 from Volker Fröhlich volker27@gmx.at 2011-05-14 15:01:13 EDT --- Please also delete the 4 pre-built binaries in the prep section!
http://fedoraproject.org/wiki/Packaging/Guidelines#No_inclusion_of_pre-built...
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=697836
--- Comment #7 from Adam Huffman bloch@verdurin.com 2011-05-27 10:13:59 EDT --- New version at:
http://verdurin.fedorapeople.org/reviews/mmseq/mmseq.spec
http://verdurin.fedorapeople.org/reviews/mmseq/mmseq-0.9.10b-3.fc16.src.rpm
I patched the Makefile as you suggested but without the zlib-devel BR builds in Mock failed.
I will be building for EPEL and samtools will be there soon (I maintain the EPEL branches).
The upstream author explained that 0.9.10b is a post-release update, which according to the guidelines at http://fedoraproject.org/wiki/PackageNamingGuidelines#NonNumericRelease is acceptable as is.
I've removed VERSION.
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=697836
--- Comment #8 from Adam Huffman bloch@verdurin.com 2011-05-27 10:24:12 EDT --- The wrong version was uploaded - I've fixed that now.
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=697836
--- Comment #9 from Adam Huffman bloch@verdurin.com 2011-05-28 06:23:11 EDT --- New upstream release:
http://verdurin.fedorapeople.org/reviews/mmseq/mmseq.spec
http://verdurin.fedorapeople.org/reviews/mmseq/mmseq-0.9.11-1.fc16.src.rpm
I've reminded the upstream author about the incorrect FSF addresses.
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=697836
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #10 from Volker Fröhlich volker27@gmx.at 2011-05-28 09:47:53 EDT --- As the name issue is solved, the package is fine.
md5sum: 6c75e6866eb702b7b22892b63340280e
http://koji.fedoraproject.org/koji/taskinfo?taskID=3097653
======== APPROVED ========
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=697836
Adam Huffman bloch@verdurin.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #11 from Adam Huffman bloch@verdurin.com 2011-05-29 10:42:26 EDT --- New Package SCM Request ======================= Package Name: mmseq Short Description: Haplotype and isoform specific expression estimation for RNA-seq Owners: verdurin Branches: f14 f15 el5 el6 InitialCC:
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=697836
--- Comment #12 from Jason Tibbitts tibbs@math.uh.edu 2011-05-29 11:21:42 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=697836
Adam Huffman bloch@verdurin.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ON_QA
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=697836
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |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=697836
--- Comment #13 from Fedora Update System updates@fedoraproject.org 2011-06-17 11:26:52 EDT --- mmseq-0.9.11-1.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/mmseq-0.9.11-1.el5
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=697836
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #14 from Fedora Update System updates@fedoraproject.org 2011-06-21 13:13:46 EDT --- mmseq-0.9.11-1.el5 has been pushed to the Fedora EPEL 5 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=697836
--- Comment #15 from Fedora Update System updates@fedoraproject.org 2011-07-12 11:00:07 EDT --- mmseq-0.9.11-1.el5 has been pushed to the Fedora EPEL 5 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=697836
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |mmseq-0.9.11-1.el5 Resolution| |ERRATA Last Closed| |2011-07-12 11:00:14
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla