Bug 480102 - Review Request: weka - Waikato Environment for Knowledge Analysis
Review Request: weka - Waikato Environment for Knowledge Analysis
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michel Alexandre Salim
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-14 20:02 EST by Milos Jakubicek
Modified: 2009-03-09 19:03 EDT (History)
3 users (show)

See Also:
Fixed In Version: 3.6.0-3.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-09 19:02:38 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
michel: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Milos Jakubicek 2009-01-14 20:02:30 EST
Spec URL: http://mjakubicek.fedorapeople.org/weka/weka.spec
SRPM URL: http://mjakubicek.fedorapeople.org/weka/weka-3.6.0-1.fc10.src.rpm
Description: Weka is a collection of machine learning algorithms for data mining tasks. The algorithms can either be applied directly to a dataset or called from
your own Java code. Weka contains tools for data pre-processing, classification, regression, clustering, association rules, and visualization. It is also well-suited for developing new machine learning schemes.

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1054196
Comment 1 Michel Alexandre Salim 2009-02-25 10:47:25 EST
I'm actually using Weka right now, so I'll take the review. Will post review notes in a bit.
Comment 2 Michel Alexandre Salim 2009-02-25 15:31:08 EST
General guidelines:

MUST

+ rpmlint
+ package name
+ spec file name
+ package guideline-compliant
+ license complies with guidelines
+ license field accurate
+ license file not deleted
+ spec in US English
+ spec legible
+ source matches upstream
+ builds under >= 1 archs, others excluded
  noarch
+ build dependencies complete
N/A locales handled using %find_lang, no %{_datadir}/locale
+ own all directories
+ no dupes in %files
+ permission
+ %clean RPM_BUILD_ROOT
+ macros used consistently
+ Package contains code
+ large docs => -doc (-javadoc)
+ doc not runtime dependent

- desktop file uses desktop-file-install

  Desktop file uses desktop-file-install, but the path to the icon is wrong:
  /usr/share/icons/weka.ico rather than just
  1. weka (which is preferable) or 
  2. the correct path (/usr/share/icons/hicolor/yadda yadda)

  It looks like .ico files are not automatically picked up by GNOME's menu,
  so I'd consider converting the icon to .png (it makes it smaller as well),
  in which case you can just refer to it as Icon=weka. Otherwise, you'd have
  to use the complete path

  Also, icon caches are not updated:
  http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GTK.2B_icon_cache


+ clean buildroot before install
• filenames UTF-8

SHOULD
+ package build in mock on all architectures
+ package functioned as described
+ scriplets are sane
+ other subpackages should require versioned base
+ require package not files

Java-specific:
http://fedoraproject.org/wiki/Packaging/Java

- Java requirement must be versioned (see BuildRequires and Requires)

Otherwise, it looks fine, so I can approve this after the fixes are made.
Comment 3 Milos Jakubicek 2009-02-26 20:25:55 EST
* Wed Feb 27 2009 Milos Jakubicek <xjakub@fi.muni.cz> - 3.6.0-2
- Running gtk-update-icon-cache
- Converting icon to png, added BR:ImageMagick, changed icon path
  in desktop file "weka" only
- java/java-devel R: and BR: made versioned to >= 1:1.6.0
- removed vendor tag in desktop-file-install according to the guidelines
  for new packages.

New SRPM: http://mjakubicek.fedorapeople.org/weka/weka.spec
New SPEC: http://mjakubicek.fedorapeople.org/weka/weka-3.6.0-2.fc10.src.rpm
Comment 4 Michel Alexandre Salim 2009-02-27 01:36:33 EST
Most of the changes are fine. Would be a good idea to turn on the test suite, between %build and %install:

%check
CLASSPATH=/usr/share/java/junit4.jar ant run_tests

This currently fails, though:
run_tests:
    [junit] Error: java.lang.ClassCastException: class weka.AllTests

not sure why, you might want to ask on the upstream mailing list.

Oh, and the %files section needs to be updated: it's still refering to fedora-%{name}.desktop.
Comment 5 Michel Alexandre Salim 2009-02-27 10:11:48 EST
(In reply to comment #4)
> Most of the changes are fine. Would be a good idea to turn on the test suite,
> between %build and %install:
> 
> %check
> CLASSPATH=/usr/share/java/junit4.jar ant run_tests
> 
Of course, that should have been

BuildRequires: junit

%check
CLASSPATH=/usr/share/java/junit.jar ant run_tests

It takes forever to run, though, so perhaps surround it in an %if?

%if %{?_with_tests:1}%{!?_with_tests:0}
%check
...
%endif

(and put a one-line comment at the top describing the --with tests option)

But that's secondary -- apart from the desktop file naming, this is good to go.
Comment 6 Milos Jakubicek 2009-02-28 11:18:29 EST
* Sat Feb 28 2009 Milos Jakubicek <xjakub@fi.muni.cz> - 3.6.0-3
- Fixed desktop file name
- Added optional running junit tests into %%check, added BR:junit
- Fixed changelog entry

The failing jUnit tests have been files into upstream bugtracker.

New SRPM: http://mjakubicek.fedorapeople.org/weka/weka.spec
New SPEC: http://mjakubicek.fedorapeople.org/weka/weka-3.6.0-3.fc10.src.rpm
Comment 7 Michel Alexandre Salim 2009-02-28 12:25:32 EST
Changes look good.

APPROVED
Comment 8 Milos Jakubicek 2009-02-28 17:38:15 EST
Michel, thanks for review!

New Package CVS Request
=======================
Package Name: weka
Short Description: Waikato Environment for Knowledge Analysis
Owners: mjakubicek
Branches: F-9 F-10
InitialCC:
Comment 9 Kevin Fenzi 2009-02-28 18:41:33 EST
cvs done.
Comment 10 Fedora Update System 2009-03-01 11:36:23 EST
weka-3.6.0-3.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/weka-3.6.0-3.fc9
Comment 11 Fedora Update System 2009-03-01 11:37:01 EST
weka-3.6.0-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/weka-3.6.0-3.fc10
Comment 12 Fedora Update System 2009-03-04 11:20:54 EST
weka-3.6.0-3.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update weka'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-2276
Comment 13 Fedora Update System 2009-03-04 11:23:20 EST
weka-3.6.0-3.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing-newkey update weka'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-2299
Comment 14 Fedora Update System 2009-03-09 19:02:33 EDT
weka-3.6.0-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 15 Fedora Update System 2009-03-09 19:03:37 EDT
weka-3.6.0-3.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

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