[Bug 611454] Review Request: python-bcrypt - Python bindings for OpenBSD's Blowfish password hashing code

[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=611454

Mohammed Safwat <Mohammed_ElAfifi at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Mohammed_ElAfifi at yahoo.com

--- Comment #7 from Mohammed Safwat <Mohammed_ElAfifi at yahoo.com> 2010-08-07 19:56:18 EDT ---
I amn't a sponsor; this's just a casual review.

- It's absolutely legal to define the variable real_name with %global, but
since it's only designated once with the %setup macro, you can really directly
substitute the name py-bcrypt directly into %setup.

- Somehow related to the above remark, if the software you're packaging is
really called py-bcrypt, why do you refer to it in the Name tag and the
description section as python-bcrypt? I mean you can use the name py-bcrypt
everywhere in the file wherever it's needed.

- The software website shows a newer version(0.2, quite recently released) than
the one you're trying to package here; maybe you should check to see if you
want to package that latest version instead.

- The BuildRequires tag should refer to python2-devel, python3-devel, or
both(if the package modules are built against both python 2 and 3). The
BuildRequires tag currently shows python-devel, which doesn't indicate which
python to build the package against. See
https://fedoraproject.org/wiki/Packaging:Python#BuildRequires. From the %files
section I see that you're generating an egg package for python 2.6; this might
dictate that you use python2-devel.

- CFLAGS="%{optflags}" in %build section is superfluous as it's typically used
by c/c++ compilers by default. Check
http://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags.

- Every package MUST have a changelog section to show the history of changes it
underwent. There's no such changelog currently in the package. Check
http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs.

-- 
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.