This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 722713 - Review Request: pyace - Optimal route search in a complete graph
Review Request: pyace - Optimal route search in a complete graph
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
17
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2011-07-16 18:55 EDT by Mauricio Cleveland
Modified: 2013-01-10 18:10 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-01-10 18:10:05 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Mauricio Cleveland 2011-07-16 18:55:02 EDT
Spec URL: http://curvatura.googlecode.com/files/ace.spec
SRPM URL: http://curvatura.googlecode.com/files/ace-1.1-2.fc15.src.rpm
Description: Implementation of an algorithm based on the spatial curvature.
Comment 1 Luis Bazan 2011-07-20 15:46:13 EDT
Please remember to incluse rpmlint on your requests.

$ rpmlint ace-1.1-2.fc15.src.rpm 
ace.src: W: invalid-license GPL
ace.src: W: invalid-url Source0: http://curvatura.googlecode.com/files/ace-1.1.tar.gz HTTP Error 404: Not Found
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

invalid license GPL

check URL 

http://fedoraproject.org/wiki/Licensing#SoftwareLicenses

change to GPLv2+

your %doc is empty.
I think you should add README

change to -> %doc README

https://fedoraproject.org/wiki/Packaging:Python
Comment 2 Mauricio Cleveland 2011-07-21 21:06:54 EDT
(In reply to comment #1)
> Please remember to incluse rpmlint on your requests.
> 
> $ rpmlint ace-1.1-2.fc15.src.rpm 
> ace.src: W: invalid-license GPL
> ace.src: W: invalid-url Source0:
> http://curvatura.googlecode.com/files/ace-1.1.tar.gz HTTP Error 404: Not Found
> 1 packages and 0 specfiles checked; 0 errors, 2 warnings.
> 
> invalid license GPL
> 
> check URL 
> 
> http://fedoraproject.org/wiki/Licensing#SoftwareLicenses
> 
> change to GPLv2+
> 
> your %doc is empty.
> I think you should add README
> 
> change to -> %doc README
> 
> https://fedoraproject.org/wiki/Packaging:Python

Ok, corrections:

SPEC URL: http://curvatura.googlecode.com/files/ace.spec
SRPM URL: http://curvatura.googlecode.com/files/ace-1.5-2.fc15.src.rpm

rpmlint:

[MakeRPM@Quasard SRPMS]$ rpmlint ace-1.5-2.fc15.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 3 Nathan Owe 2011-07-26 16:23:23 EDT
%defattr line is no longer needed unless you are building for the RPEL 

Remove Requires:
http://fedoraproject.org/wiki/Packaging:Guidelines#Requires
Since RPM will find the dependency needed automaticly.

BuildRoot line is not required unless building for the EPEL
Also each time you update the SPEC file, bump up the release and note the change in the Changelog

According to the homepage, the package is GPLv3+

Also the %clean section isn't needed except for EPEL
Prefix: tag isn't needed either

Running rpmlint on the RPM package(not SRPM) gives that you need a man page for it as well

Description is too long
http://fedoraproject.org/wiki/Common_Rpmlint_issues#description-line-too-long

This is how mine would look if I did it, except with my name and email of course:
http://github.com/ndowens/Fedora-Rpms/raw/master/SPECS/ace.spec

The only issue that rpmlint gives me is that a man page is needed
Comment 4 Mauricio Cleveland 2011-08-17 01:59:24 EDT
(In reply to comment #3)
> %defattr line is no longer needed unless you are building for the RPEL 
> 
> Remove Requires:
> http://fedoraproject.org/wiki/Packaging:Guidelines#Requires
> Since RPM will find the dependency needed automaticly.
> 
> BuildRoot line is not required unless building for the EPEL
> Also each time you update the SPEC file, bump up the release and note the
> change in the Changelog

OK.

> 
> According to the homepage, the package is GPLv3+

OK.

> 
> Also the %clean section isn't needed except for EPEL
> Prefix: tag isn't needed either
> 

OK.

> Running rpmlint on the RPM package(not SRPM) gives that you need a man page for
> it as well

bash-4.2$ rpmlint ace-1.5-4.fc15.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

> 
> Description is too long
> http://fedoraproject.org/wiki/Common_Rpmlint_issues#description-line-too-long
> 

OK.

> This is how mine would look if I did it, except with my name and email of
> course:
> http://github.com/ndowens/Fedora-Rpms/raw/master/SPECS/ace.spec
> 
> The only issue that rpmlint gives me is that a man page is needed

OK.

http://code.google.com/p/curvatura/source/browse/ace.spec
http://code.google.com/p/curvatura/source/browse/RPMS/ace-1.5-4.fc15.noarch.rpm
http://code.google.com/p/curvatura/source/browse/RPMS/ace-1.5-4.fc15.src.rpm

I hope this all right.
Comment 5 Volker Fröhlich 2011-08-22 15:53:21 EDT
Changelog in English, please.
Comment 6 Mauricio Cleveland 2011-09-13 15:18:48 EDT
Ready!
(In reply to comment #5)
> Changelog in English, please.
Comment 7 Volker Fröhlich 2011-09-16 17:11:42 EDT
(The changelog is actually only for changes related to packaging. Don't mix that with changes in the software. These belong in the update comment later, when you push the package.)
Comment 8 Volker Fröhlich 2011-09-17 11:28:03 EDT
There are loads of strings in Spanish in the program. All comments are Spanish as well. I recommend to change that, but that has nothing to do with packaging. The manpage contains "Para salir presione la letra "q".", which is not useful at all.

There are several spelling mistakes over the place, like "coma", "aceleration" or "Numer" all over the place.

The package name must obey http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29

The Readme says GPLv3+, your Spec says 2+.
Comment 9 Kevin Kofler 2011-09-17 12:10:10 EDT
Can you please give this package a less generic name? There are already many pieces of software calling themselves "ace", including, but not limited to:
* a proprietary packer,
* the ADAPTIVE Communication Environment: http://www.cs.wustl.edu/~schmidt/ACE.html
* a "boot time configuration engine for appliances" which was previously in Fedora (but has been retired): https://admin.fedoraproject.org/pkgdb/acls/name/ace
etc.
Comment 10 Volker Fröhlich 2011-12-13 14:37:04 EST
Mauricio, any news here?
Comment 11 Mauricio Cleveland 2011-12-15 10:04:03 EST
Yes, i am working in the theorical paper for presentation in a congress. After this i continue the RPM implementation.

The paper is in spanish here: http://www.universodigital.cl/wp-content/uploads/2011/09/curvatura1.pdf
Comment 12 Kevin Kofler 2011-12-15 13:53:13 EST
In any case, both the package and the executable need to get some longer, less ambiguous name than "ace" (see comment #9). (If you can't think of anything more suitable, you can call them algoritmo-curvatura-espacial, which is both a valid RPM name and a valid executable name in Fedora.)
Comment 13 Kevin Kofler 2011-12-15 14:00:12 EST
In case Volker or anybody else wonders: The paper is about solving the traveling salesman problem by a heuristic which simulates space-time curvature. (Thankfully, since I speak Italian and French fluently, I can make sense of Spanish.)
Comment 14 Mauricio Cleveland 2012-02-16 22:16:57 EST
Fine, the code was full rewrited. The translation is complete too and the name is changed.
Comment 16 Volker Fröhlich 2012-02-21 15:45:02 EST
SRPM and Spec file are enough, don't submit the final package.

The spec file must be called the same as package, as rpmlint will tell you.

Please submit the files in a way they can be easily downloaded. Right now, these are not direct links. You can neither install with rpm -ivh this way, nor will fedora-review work.

defattr is no longer necessary.

Please handle the locales as described in the packaging guidelines.
Comment 17 Mauricio Cleveland 2012-03-08 14:45:09 EST
(In reply to comment #16)
> SRPM and Spec file are enough, don't submit the final package.

http://curvatura.googlecode.com/files/pyace-3.0-2.fc16.src.rpm
http://curvatura.googlecode.com/files/pyace.spec

> 
> The spec file must be called the same as package, as rpmlint will tell you.
> 

[cleve@Quasard src]$ rpmlint pyace-3.0-2.fc16.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[cleve@Quasard curvatura]$ rpmlint pyace.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

> Please submit the files in a way they can be easily downloaded. Right now,
> these are not direct links. You can neither install with rpm -ivh this way, nor
> will fedora-review work.

Sorry, my mistake.

> 
> defattr is no longer necessary.

Ok.

> 
> Please handle the locales as described in the packaging guidelines.

And Ok.
Comment 18 Volker Fröhlich 2012-03-08 15:25:54 EST
README: "pyACE is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

...

http://code.google.com/p/curvatura/
"

Spec file: GPLv2+
http://curvatura.googlecode.com/

pyace.noarch: W: incoherent-version-in-changelog 3.2 ['3.0-2.fc16', '3.0-2']

The examples should be labeled documentation. It is common to put the documentation first in the files section.

The manpage should rather be %{_mandir}/man1/pyace.1*

Please use the name macro consistently, meaning don't switch between %{name} and pyace.

I think it'd be better to have %{python_sitelib}/%{name}-%{version}-py*.egg-info instead of a specific Python version.

I'd write .py* instead of .py, .pyc and .pyo, but it's not a blocker.

There's a lot of slack in the tarball, by the way:
- rpms and tarball in src
- .git stuff

Take a look at http://linux.die.net/man/1/git-archive for best practice on how to release a tarball with Git.
Comment 19 Volker Fröhlich 2012-04-10 17:01:19 EDT
Any news here?
Comment 20 Mauricio Cleveland 2012-05-06 23:09:56 EDT
(In reply to comment #19)

Yes, new version, let see.

http://curvatura.googlecode.com/files/pyace-4.0-1.fc16.src.rpm

http://curvatura.googlecode.com/files/pyace.spec

cleve ->curvatura$ rpmlint pyace.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

I hope this time the package can be accepted, if not, some day! 

> Any news here?
Comment 21 Volker Fröhlich 2012-05-14 17:51:21 EDT
I'm afraid you can't install %{python_sitelib}/operations and %{python_sitelib}/optimization. This is very generic. Install your package in one sub-directoy, for instance  %{python_sitelib}/%{name}. You can use that in the files section than, which simplifies it a lot.

There are a couple of typos/mistakes in README and the manpages, for instance:

"pyACE is a program that resolve complete" -- That should be "resolves", I guess.
"See the examples or visit the website for know how write the input files."

"the Free Software Foundation, either version 2+ of the License, or
(at your option) any later version." -- That doesn't make sense. That should be "either version 2 of the License ..."

"You should have received a copy of the GNU General Public License
along with ACE.  If not, see <http://www.gnu.org/licenses/>." -- You don't ship a copy. Please add one.

The changelog of the spec file is intended for changes in packaging. Don't use it to describe changes in your software.

Why are you running find_lang twice?

The description should be more detailled than the summary, see: http://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description

I'd put an asterisk for the Python version number in %{python_sitelib}/%{name}-%{version}-py2.7.egg-info.

Please ship the examples by using the doc macro.
Comment 22 Kevin Kofler 2012-05-14 18:22:51 EDT
> "the Free Software Foundation, either version 2+ of the License, or
> (at your option) any later version." -- That doesn't make sense. That should be
> "either version 2 of the License ..."

To be clear, the correct wording is:
"the Free Software Foundation, either version 2 of the License, or
(at your option) any later version."
(There is no "version 2+", 2+ is an informal shortcut for "2 or later".)
Comment 23 Volker Fröhlich 2012-07-22 16:20:19 EDT
How's it going here?
Comment 24 Mauricio Cleveland 2012-07-25 17:08:47 EDT
I am working with a mentor. I am making the RPM for new version.
Comment 25 Volker Fröhlich 2012-09-30 14:22:23 EDT
Any news?
Comment 26 François Cami 2013-01-10 18:10:05 EST
Mauricio is marked inactive in FAS and has not answered for more than a week.
Closing (see https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews).

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