[Bug 1128253] Review Request: gerrymander - A client API and command line tool for gerrit

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

 



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



--- Comment #4 from Kashyap Chamarthy <kchamart@xxxxxxxxxx> ---
Thank you Parag, for the review, and your help on IRC. I addressed all your
comments (please see inline).

Updated SPEC: https://kashyapc.fedorapeople.org/spec-files/gerrymander.spec
Updated SRPM:
https://kashyapc.fedorapeople.org/srpms/gerrymander-1.3-2.fc20.src.rpm

(In reply to Parag AN(पराग) from comment #3)
> First few things to fix
> 1) We use %global over %define. Fix this

Done - Replaced %define with %global

> 2) If not defined macro "%{?extra_release}" then we don't even need it. You
> can remove this.

Removed the "%{?extra_release}" macro.

> 3) Group tag is optional but can be kept if building for EPEL

I left the "Group" tag intact, in case we may have to build this for EPEL.

> 4) Replace python with python2 wherever it applies.

Done - s/python/python2

> 5) Please have correct summary matching to package name

Corrected summary for python2 package description.

> 6) use python2 macros wherever applies

Done - used python2 macros consistently wherever it applies.

> 7) defattr is not needed 

Removed %defattr section from %files, as it is now implied by default.

> 8) This package is not compiling anything so we don't need
> CFLAGS="$RPM_OPT_FLAGS" in %build

Done - Removed it.

> 9) If upstream provides egginfo in source we need to remove it in %prep and
> package built egginfo in %install. You are removing it actually in %install

Done - Changed egg info removal section from %install to %prep

> 
> 10) use source url as
> Source0: https://github.com/berrange/%{name}/archive/v%{version}.tar.gz

Done.

> 
> Please provide updated package for further review and check the
> https://fedoraproject.org/wiki/Packaging:Python page. This package spec is
> not following those guidelines.

Please review the updated SPEC to see if it complies the guidelines now.

-- 
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
https://admin.fedoraproject.org/mailman/listinfo/package-review





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