Bug 239282 - Review Request: seaview - Graphical multiple sequence alignment editor
Review Request: seaview - Graphical multiple sequence alignment editor
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-07 08:31 EDT by Christian Iseli
Modified: 2007-11-30 17:12 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-05 18:34:49 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Christian Iseli 2007-05-07 08:31:58 EDT
Spec URL: ftp://ftp.licr.org/pub/seaview.spec
SRPM URL: ftp://ftp.licr.org/pub/seaview-0.20070417-0.src.rpm
Description:
SeaView is a graphical multiple sequence alignment editor developed by Manolo
Gouy.  SeaView is able to read and write various alignment formats (NEXUS, MSF,
CLUSTAL, FASTA, PHYLIP, MASE).  It allows to manually edit the alignment, and
also to run DOT-PLOT or CLUSTALW/MUSCLE programs to locally improve the
alignment.


Another bioinformatics tool.  The code is under the GPL, though the author
didn't include the GPL license itself in the source tarball (the source
files are clearly marked though).

Another minor annoyance is that there is no version info anywhere...
so I used the date, which is what the OA seems to do as well.

The code can optionnaly make use of the PDF-libs, but since this apparently
can't go in Fedora, this package compiles using the PostScript output.

Compiles in mock, has a clean rpmlint output, and WorksForMe(tm) :-)
Comment 1 Bernard Johnson 2007-05-08 00:53:05 EDT
(In reply to comment #0)
> SRPM URL: ftp://ftp.licr.org/pub/seaview-0.20070417-0.src.rpm

I can't find anything in the packaging guidelines that specifically address "no
version", so I could be wrong when I say this...

To me it seems pointless to prefix the date with a 0. in this case.  You will
never have a number low enough that upstream can't trump you and release
something like 0.01 which would be < EVR than yours.  I would think this would
be the perfect case for an epoch if upstream gets a clue and versions the
package at a later date.
Comment 2 Christian Iseli 2007-05-08 03:29:12 EDT
(In reply to comment #1)
> To me it seems pointless to prefix the date with a 0. in this case.  You will
> never have a number low enough that upstream can't trump you and release
> something like 0.01 which would be < EVR than yours.  I would think this would
> be the perfect case for an epoch if upstream gets a clue and versions the
> package at a later date.

This got me thinking, and I had another look at what we currently have in FC-6.
I think it would actually make more sense to put zero as a version number, and
follow the pre-release snapshot guidelines for the release number.  This way,
when upstream so decides, there is a very good probability that the version
assigned will be higher than zero (0) and we avoid the epoch thing.  Deal ?

new SPEC: ftp://ftp.licr.org/pub/seaview.spec
new SRPM: ftp://ftp.licr.org/pub/seaview-0-0.20070417.src.rpm
Comment 3 Jason Tibbitts 2007-06-12 18:19:16 EDT
This has been sitting for a while and looks kind of interesting; I'll take a look.

The naming guidelines actually cover this situation with an example

  Release Tag for Pre-Release Packages:
    * 0.%{X}.%{alphatag}
  Example (pre-release svn checkout):
    kismet-0-0.1.20040110svn (this is a pre-release, svn checkout of kismet)
    kismet-0-0.2.20040110svn (this is a bugfix to the previous package)
(etc.)

So I guess another digit is in order.

One fundamental problem I see is that the tarball doesn't match what I get from
upstream.  It looks like there's been another release.  Can you verify?  (Three
chears for insane upstreams who don't bother to version anything.)
Comment 4 Christian Iseli 2007-06-13 02:30:15 EDT
(In reply to comment #3)
> This has been sitting for a while and looks kind of interesting; I'll take a look.

Thanks tibbs

> So I guess another digit is in order.

Right.  I guess I need to learn to read some day :-)
Fixed

> One fundamental problem I see is that the tarball doesn't match what I get from
> upstream.  It looks like there's been another release.  Can you verify?  (Three
> chears for insane upstreams who don't bother to version anything.)

Yea, upstream put a new "version"... /me cries...

Here's the updated stuff:
SPEC: ftp://ftp.licr.org/pub/seaview.spec
SRPM: ftp://ftp.licr.org/pub/seaview-0-0.1.20070515.fc7.src.rpm
Comment 5 Jason Tibbitts 2007-06-27 22:20:56 EDT
Sorry for not getting back to this sooner.  It looks like I need to be quicker
or something else is amiss, because I'm still seeing a different upstream
tarball that what's in this package.

Also, this does seem to be a graphical application and so probably deserves a
.desktop file.  An icon is even included in the tarball.
Comment 6 Christian Iseli 2007-06-28 09:51:00 EDT
Yup, tarball changed again...

I added a desktop file.

Here's the updated stuff:
SPEC: ftp://ftp.licr.org/pub/seaview.spec
SRPM: ftp://ftp.licr.org/pub/seaview-0-0.1.20070615.fc7.src.rpm
Comment 7 Jason Tibbitts 2007-06-28 10:59:13 EDT
I'd better get to this one fast before the tarball changes again....
Comment 8 Jason Tibbitts 2007-06-28 12:03:16 EDT
OK, the only issue I see is that we don't use the X-Fedora category these days,
so you shouldn't have the "--add-category" bit.

But that's really minor; you can fix it when you check in.

APPROVED

Review:
* source files match upstream:
   ce08adfd4f177082c6ff9eb049d4405a9db0ed9383c2f41705b1c8a719036880
   seaview.tar
* 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.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
   seaview = 0-0.1.20070615.fc8
  =
   libX11.so.6()(64bit)
   libfltk.so.1.1()(64bit)
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)
* %check is not present (or at least it's empty); no test suite upstream.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.
X desktop file is present and looks OK but uses X-Fedora category.
Comment 9 Christian Iseli 2007-06-28 15:31:26 EDT
Thanks tibbs

(In reply to comment #8)
> OK, the only issue I see is that we don't use the X-Fedora category these days,
> so you shouldn't have the "--add-category" bit.

Hmm, I supposed I should have snarfed the .desktop file and spec bits from
another package :)
I'll fix it.
A quick grep for X-Fedora in all the devel/*.spec files turns about 400 matches,
though about half of them are changelog entries about the removal of X-Fedora.

Still, I think we have about 200 packages that need fixing.  If I find some
time, I'll try to mass-BZ the issue.
Comment 10 Christian Iseli 2007-06-28 15:36:00 EDT
New Package CVS Request
=======================
Package Name: seaview
Short Description: Graphical multiple sequence alignment editor
Owners: Christian.Iseli@licr.org
Branches: FC-6 F-7
InitialCC: 
Comment 11 Kevin Fenzi 2007-06-28 16:03:29 EDT
cvs done.
Comment 12 Christian Iseli 2007-07-05 18:34:49 EDT
imported and built.  Thanks for the review :)

Note You need to log in before you can comment on or make changes to this bug.