https://bugzilla.redhat.com/show_bug.cgi?id=1081434 Christopher Meng <i@xxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|Reopened | --- Comment #27 from Christopher Meng <i@xxxxxxxx> --- (In reply to Guruswamy Basavaiah from comment #26) > If no more comments, could this review request for package be approved ? Why do you think so? Is this package perfect? Absolutely not. 1. Source0: http://www.ip2location.com/downloads/ip2location-c-6.0.3.tar.gz If you don't want to make it harder, please use this: Source0: http://www.ip2location.com/downloads/ip2location-c-%{version}.tar.gz 2. The above one is cosmetic, however the below context is really problematic %package data Summary: Country DB for IP2Location C library Group: Applications/Databases Requires: %{name}%{?_isa} = %{version}-%{release} %description data IP2Location Country DB contains IP address to country mapping. DB can be accessed with IP2Location C library. The reason you split should be reducing the size of the package, however if you don't mark it as BuildArch: noarch, I don't see any benefits here. And if you mark it as noarch, this line should be modified: Requires: %{name}%{?_isa} = %{version}-%{release} to Requires: %{name} = %{version}-%{release} 3. What's the license of the BIN data? 4. %doc Developers_Guide.txt LICENSE.GPLv3 LICENSE.LGPLv3 I disagree of putting the dev guide into main package. You should put it in the -devel subpkg. 5. You declared that this package is License: LGPLv3, as mentioned above, there are 2 license file in the main package, superfluous or mistake? 6. Is the -data needed for the runtime library? If so you should add Requires in the main package, if not you should modify you %description of main package: IP-COUNTRY LITE database file that is accurate as of April 2014. Users can download the latest LITE database from IP2Location web site. 7. You can drop Group tag if you don't want to support RHEL5 (I guess so). 8. I couldn't find any info of version 6.0.3 from http://www.ip2location.com/developers/c so far. 9. You should take some reviews as informal reviews to prove yourself competent to be a packager. But that's up to the reviewer(same as your sponsor since it's your first package). -- 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