[Bug 1382989] Review Request: mirrormanager2 - Mirror management application

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

 



https://bugzilla.redhat.com/show_bug.cgi?id=1382989

Neal Gompa <ngompa13@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ngompa13@xxxxxxxxx



--- Comment #4 from Neal Gompa <ngompa13@xxxxxxxxx> ---
Some notes:

* The conditional on the python-sqlalchemy BuildRequires and Requires is
redundant, as "0%{?rhel} == 6" implies both the variable's existence and the
value must be "6". I suggest simplifying it to "%if 0%{?rhel} == 6".

* The %python_sitelib macro definition is unnecessary and can be stripped.

* Add "python-srpm-macros" and "python2-rpm-macros" as BuildRequires (for
%{?rhel} builds only) so you can use the standard Fedora %py2_build,
%py2_install, and %python2_sitelib macros. That brings it more in-line with
current packaging practices. These macros are available for EL6+ through these
packages. Fedora pulls them in through the python{2,3}-devel subpackage
automatically.

* Remove the "rm -rf $RPM_BUILD_ROOT" command in %install, as it's not needed.

* Using "%{buildroot}" is preferred over "$RPM_BUILD_ROOT".

* If you're going to "Requires: %{name}-lib" be constrained by the same
version, you might as well tighten it to %{version}-%{release}, to prevent
issues when patches might affect that part. Also, using "==" there is
misleading, as it's not a comparison, but an assignment. Use "=".

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




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