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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |hdegoede@xxxxxxxxxx
         AssignedTo|nobody@xxxxxxxxxxxxxxxxx    |hdegoede@xxxxxxxxxx
               Flag|                            |fedora-review?




--- Comment #23 from Hans de Goede <hdegoede@xxxxxxxxxx>  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.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]