Bug 1076186 - Review Request: openstv - single transferable vote and instant runoff voting software
Summary: Review Request: openstv - single transferable vote and instant runoff voting ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ankur Sinha (FranciscoD)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-03-13 18:12 UTC by David King
Modified: 2014-05-28 23:54 UTC (History)
2 users (show)

Fixed In Version: openstv-1.7-1.fc20
Clone Of:
Environment:
Last Closed: 2014-05-28 23:53:00 UTC
Type: ---
Embargoed:
sanjay.ankur: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description David King 2014-03-13 18:12:34 UTC
Spec URL: http://amigadave.fedorapeople.org/openstv.spec
SRPM URL: http://amigadave.fedorapeople.org/openstv-1.7-1.fc20.src.rpm
Description: OpenSTV is an open-source software for implementing the single transferable vote and other voting methods such as instant runoff voting, Condorcet voting, and approval voting. OpenSTV is the only open-source software that implements the single transferable vote exactly as used by governments, including Scotland and the City of Cambridge, Massachusetts. These methods have been extensively verified against other software and/or actual election results.

Organizations can use OpenSTV to implement their own elections. First, the organization must adopt a voting method. Second, the organization must conduct the vote, and this will most likely be done with paper ballots. Third, the ballots must be entered into the OpenSTV program. Finally, you can use OpenSTV to count the votes and determine the winners of the election.
Fedora Account System Username: amigadave

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6630458
Test election data: https://vote.gnome.org/blt.php?election_id=21

Comment 1 Ankur Sinha (FranciscoD) 2014-04-15 11:53:32 UTC
I'll review this one.

Comment 2 David King 2014-05-15 11:57:24 UTC
Hi Ankur, do you have time to review the package? As it has been 1 month since your last comment, I will wait a week before resetting the review flag back to empty, following https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews#Reviewer_not_responding (but I would rather get the package reviewed, if you have the time). Thanks!

Comment 3 Ankur Sinha (FranciscoD) 2014-05-15 14:32:02 UTC
Hi David,

I'll review it by the end of the week. I seem to have forgotten to add it to my work list. Apologies.

Warm regards,
Ankur

Comment 4 Ankur Sinha (FranciscoD) 2014-05-18 04:40:21 UTC
[+] OK
[-] NA
[?] Issue

** Mandatory review guidelines: **
 [+] rpmlint output:
  
[asinha@ankur-laptop  SRPMS]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm ../SPECS/openstv.spec ./openstv-1.7-1.fc20.src.rpm
openstv.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/openstv/License.html
openstv.src: W: strange-permission openstv-1.7-setup-package-data.patch 0600L
openstv.src: W: strange-permission openstv-1.7-setup-package-data.patch 0600L
3 packages and 1 specfiles checked; 1 errors, 2 warnings.
[asinha@ankur-laptop  SRPMS]$

Please inform upstream of the wrong address.

 [+] License is acceptable (...)
 [+] License field in spec is correct
 [?] License files included in package %docs if included in source package
The License isn't installed as a doc. It's installed with the rest of the
python files. Does it need to be placed there for functioning of the software?
Otherwise, please consider shifting it to docdir.

 [-] License files installed when any subpackage combination is installed
 [-] Spec written in American English
 [+] Spec is legible
 [-] Sources match upstream unless altered to fix permissibility issues
   Upstream SHA256: ...
   Your SHA256:     ...
 [+] Build succeeds on at least one primary arch
 [+] Build succeeds on all primary arches or has ExcludeArch + bugs filed
 [+] BuildRequires correct, justified where necessary
 [-] Locales handled with %find_lang, not %_datadir/locale/*
 [-] %post, %postun call ldconfig if package contains shared .so files
 [+] No bundled libs
 [-] Relocatability is justified
 [+] Package owns all directories it creates
 [+] Package requires others for directories it uses but does not own
 [+] No duplication in %files unless necessary for license files
 [+] File permissions are sane
 [+] Package contains permissible code or content
 [-] Large docs go in -doc subpackage
 [+] %doc files not required at runtime
 [-] Static libs go in -static package/virtual Provides
 [-] Development files go in -devel package
 [-] -devel packages Require base with fully-versioned dependency, %_isa
 [+] No .la files
 [+] GUI app uses .desktop file, installs it with desktop-file-install
 [+] File list does not conflict with other packages' without justification
 [+] File names are valid UTF-8

** Optional review guidelines: **
 [?] Query upstream about including license files
Already included. Should be fine. 

 [-] Translations of description, summary
 [+] Builds in mock
 [+] Builds on all arches
 [?] Functions as described (e.g. no crashes)
Not tested. Please ensure it works.

 [+] Scriptlets are sane
 [-] Subpackages require base with fully-versioned dependency if sensible
 [-] .pc file subpackage placement is sensible
 [+] No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
 [+] Include man pages if available

Naming guidelines:
 [+] Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+
 [+] Package names are sane
 [+] No naming conflicts
 [+] Spec file name matches base package name
 [+] Version is sane
 [+] Version does not contain ~
 [+] Release is sane
 [+] %dist tag
 [+] Case used only when necessary
 [-] Renaming handled correctly

Packaging guidelines:
 [+] Useful without external bits
 [+] No kmods
 [-] Pre-built binaries, libs removed in %prep
 [+] Sources contain only redistributable code or content
 [+] Spec format is sane
 [+] Package obeys FHS, except libexecdir, /run, /usr/target
 [+] No files in /bin, /sbin, /lib* on >= F17
 [+] Programs run before FS mounting use /run instead of /var/run
 [+] Binaries in /bin, /sbin do not depend on files in /usr on < F17
 [+] No files under /srv, /opt, /usr/local
 [+] Changelog in prescribed format
 [+] No Packager, Vendor, Copyright, PreReq tags
 [+] Summary does not end in a period
 [-] Correct BuildRoot tag on < EL6
 [-] Correct %clean section on < EL6
 [+] Requires correct, justified where necessary
 [+] Summary, description do not use trademarks incorrectly
 [+] All relevant documentation is packaged, appropriately marked with %doc
 [+] Doc files do not drag in extra dependencies (e.g. due to +x)
 [-] Code compilable with gcc is compiled with gcc
 [-] Build honors applicable compiler flags or justifies otherwise
 [-] PIE used for long-running/root daemons, setuid/filecap programs
 [-] Useful -debuginfo package or disabled and justified
 [-] Package with .pc files Requires pkgconfig on < EL6
 [+] No static executables
 [-] Rpath absent or only used for internal libs
 [-] Config files marked with %config(noreplace) or justified %config
 [+] No config files under /usr
 [-] Third party package manager configs acceptable, in %_docdir
 [+] .desktop files are sane
 [?] Spec uses macros consistently
You've used $RPM_BUILD_ROOT at one location. Consider changing it to buildroot,
for consistency. 

 [+] Spec uses macros instead of hard-coded names where appropriate
 [-] Spec uses macros for executables only when configurability is needed
 [-] %makeinstall used only when alternatives don't work
 [-] Macros in Summary, description are expandable at srpm build time
 [-] Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir
 [-] No software collections (scl)
 [-] Macro files named /etc/rpm/macros.%name
 [+] Build uses only python/perl/shell+coreutils/lua/BuildRequired langs
 [+] %global, not %define
 [-] Package translating with gettext BuildRequires it
 [-] Package translating with Linguist BuildRequires qt-devel
 [-] File ops preserve timestamps
 [-] Parallel make
 [+] No Requires(pre,post) notation
 [-] User, group creation handled correctly (See Packaging:UsersAndGroups)
 [-] Web apps go in /usr/share/%name, not /var/www
 [-] Conflicts are justified
 [+] One project per package
 [+] No bundled fonts
 [?] Patches have appropriate commentary
Please comment the patch.

 [-] Available test suites executed in %check
 [-] tmpfiles.d used for /run, /run/lock on >= F15

 ** Python guidelines: **
 [+] Runtime Requires correct
 [-] Python macros declared on < EL6
 [+] All .py files packaged with .pyc, .pyo counterparts
 [+] Includes .egg-info files/directories when generated
 [-] Provides/Requires properly filtered
 [-] Code that invokes gtk.gdk.get_pixels_array() Requires numpy


Not many issues. Just cosmetics primarily. Can I please request you to include
and install an appdata file in the package for gnome-software?


Thanks,
Warm regards,
Ankur

Comment 5 David King 2014-05-18 08:28:06 UTC
(In reply to Ankur Sinha (FranciscoD) from comment #4)
>…
> Please inform upstream of the wrong address.

Sadly, upstream is no longer maintaining OpenSTV as Free Software. This is the last version using the GPL.

>  [?] License files included in package %docs if included in source package
> The License isn't installed as a doc. It's installed with the rest of the
> python files. Does it need to be placed there for functioning of the
> software?
> Otherwise, please consider shifting it to docdir.

There is a menu item in OpenSTV which shows the license: Help->License, so it needs to be installed with the rest of the Python files for the package to function correctly. Should I also add a symlink to /usr/shar/doc/openstv/COPYING (or similar)?

>  [?] Functions as described (e.g. no crashes)
> Not tested. Please ensure it works.

Yes, tested with the sample data and it works fine. :-)

>  [?] Spec uses macros consistently
> You've used $RPM_BUILD_ROOT at one location. Consider changing it to
> buildroot,
> for consistency. 

Thanks, fixed!

>  [?] Patches have appropriate commentary
> Please comment the patch.

The patch itself has a comment, but I have now added this to the spec file as well.

> Not many issues. Just cosmetics primarily. Can I please request you to
> include
> and install an appdata file in the package for gnome-software?

Thanks for the recommendation. I have included an AppData file along with the other changes.

Spec URL: http://amigadave.fedorapeople.org/openstv.spec
SRPM URL: http://amigadave.fedorapeople.org/openstv-1.7-1.fc21.src.rpm

Comment 6 Ankur Sinha (FranciscoD) 2014-05-19 04:59:08 UTC
Hi David,

A symlink should be fine, or you could just include a text version as another SOURCE. As long as the license is present in the package already, this isn't a blocker. 

The rest looks good to me. 


XXXXX APPROVED XXXXX

Thanks,
Warm regards,
Ankur

Comment 7 David King 2014-05-19 06:40:43 UTC
New Package SCM Request
=======================
Package Name: openstv
Short Description: single transferable vote and instant runoff voting software
Upstream URL: https://github.com/OpenTechStrategies/openstv
Owners: amigadave
Branches: f19 f20 epel7
InitialCC:

Comment 8 David King 2014-05-19 07:27:00 UTC
Hi Ankur, thanks for the review. I added a symlink for the license, and updated the srpm and spec file in place.

Comment 9 Gwyn Ciesla 2014-05-19 12:04:54 UTC
Git done (by process-git-requests).

Comment 10 Fedora Update System 2014-05-19 13:58:38 UTC
openstv-1.7-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/openstv-1.7-1.fc20

Comment 11 Fedora Update System 2014-05-19 13:59:13 UTC
openstv-1.7-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/openstv-1.7-1.fc19

Comment 12 Fedora Update System 2014-05-21 02:27:58 UTC
openstv-1.7-1.fc20 has been pushed to the Fedora 20 testing repository.

Comment 13 Fedora Update System 2014-05-28 23:53:00 UTC
openstv-1.7-1.fc20 has been pushed to the Fedora 20 stable repository.

Comment 14 Fedora Update System 2014-05-28 23:54:05 UTC
openstv-1.7-1.fc19 has been pushed to the Fedora 19 stable repository.


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