[Bug 466737] Review Request: matio - Library for reading/writing Matlab MAT files

[bugzilla at redhat.com]

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=466737


Hans de Goede <hdegoede at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |hdegoede at redhat.com
         AssignedTo|nobody at fedoraproject.org    |hdegoede at redhat.com
               Flag|                            |fedora-review?




--- Comment #23 from Hans de Goede <hdegoede at redhat.com>  2009-05-22 14:17:44 EDT ---
As discussed by mail, reviewing this one in return for you reviewing bug
502189.

Full review done, here are the must and should fix's I found:

Must Fix:
---------
* Remove the "%define _default_patch_fuzz 2", your patches have no fuzz, so
  its not needed (I tested on F-11)
* For the test package, the Group tag is wrong (spelling error,
  currently you have: "Develdment/Libraries"
* matio.src:18: W: non-break-space line 18

Should Fix:
-----------
* Rename the matio-1.3.3-fortanpath.patch to
  matio-1.3.3-fortranpath.patch (so add the missing r)
* libmatio -> matio in %description
* Remove these 2 commented lines from %setup please, as you've already
  solved this in another way in %build, they only confuse the reader
  (atleast they confused me):
  #To disable rpath
  #./bootstrap
* Consider renaming the manpages to mathio-<foo> and installing them
* Is it really useful to put the test files in a subpacke, wouldn't it be
better
  to run them as %check and not put them in an rpm at all ?

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.