Please point at a valid spec file and src.rpm.

* https://fedoraproject.org/wiki/Join_the_package_collection_maintainers
* https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
* https://fedoraproject.org/wiki/Package_Review_Process
* https://fedoraproject.org/wiki/Category:Package_Maintainers

(In reply to Michael Schwendt from comment #1)
> Please point at a valid spec file and src.rpm.
> 
> * https://fedoraproject.org/wiki/Join_the_package_collection_maintainers
> * https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
> * https://fedoraproject.org/wiki/Package_Review_Process
> * https://fedoraproject.org/wiki/Category:Package_Maintainers

Sorry for my mistake, and I have rectified the issues.

Please find the respective files at
Spec URL: http://www.ip2location.com/rpm/ip2location-c_6.0.2.spec
SRPM URL: http://www.ip2location.com/rpm/ip2location-c-6.0.2-1.fc20.src.rpm

Please review again.

(In reply to Chris Lim from comment #2)
> Please review again.
I see your URL:'s are pointing to upstream - So are you upstream?

Some remarks on your package:

* Source0:-URL should be an URL pointing to upstream 
 (Source0: www.ip2location.com/downloads/ip2location-c_6.0.2.tar.gz)

* The *.spec file name does not match Fedora's conventions
 (Must be ip2location-c.spec, not ...-<version>. spec)

* The upstream tarball is a mess.
- It contains binaries and temporary files which should not be part of a source-tarball.
  In case you're upstream, I'd recommend you to build tarballs with "make dist", which should automatically take care about this issue.
  In case you're not upstream, remove all of these in %prep.

- Many files carry bogus permissions.
  In case you're upstream, please fix them.
  In case you're not please fix them in %prep.

* libIP2Location.so isn't properly versioned (keyword: SONAME).
  In case you're upstream, please change this.
  In case you're not, please add an SONAME 0.0.0 to the package.

* Your spec is trying to build and ship a static libs.
  Please append --disable-static to %configure to suppress this behavior.

* Please split your package into a *-devel and a run-time package.

* You are trying to ship a *.la. This is not allowed in Fedora.
  Please rm -f this file in %install

* Your spec doesn't honor multilibs correctly (Carries hard-coded refs to /usr/lib). Replace references to /usr/lib with %{_libdir}.

* Supplying a file named /usr/include/imath.h seems a pretty bad idea to me, because the name seems too generic to me.
  I'd recommend upstream to either use a more unique/less likely to conflict name. Alternatively, a Fedora packager could install all of this package's headers into a subdir of /usr/include 
(e.g. /usr/include/IP2Loc; %configure --includedir=%{_includedir}/IP2Loc)

News here?

This is a very nice alternative to geoip.

I'm currently working on the comments from Ralf. Shall release an update once completed.

Hello, 
 I am currently working on the comments from Ralf. Could this bug be assigned to me.

(In reply to guru2018 from comment #6)
> Hello, 
>  I am currently working on the comments from Ralf. Could this bug be
> assigned to me.

Who are you?

Hello,
 I am one of developer involved in the development of ip2location library.

(In reply to guru2018 from comment #8)
> Hello,
>  I am one of developer involved in the development of ip2location library.

Ok! Hope you can continue the packaging soon. ;)

Hello  Christopher Meng,
 All the comment from you are addressed expected one and the package srpm is attached. 
 
 Comment which is not fixed is "Please split your package into a *-devel and a run-time package." 
 Instead single -devel package is created. ip2location provide only the libraries and headers for the application, hence no run time package is created. 

 Looking forward for any comments you have. 

Guru

Created attachment 876010 [details]
ip2location src rpm with fixes to comments from  Ralf Corsepius.

> ip2location provide only the libraries and headers for the application,
> hence no run time package is created. 

This is not acceptable.

The shared library is the runtime part, the headers and libIP2Location.so are the build-time part.


That's not the only packaging mistake, however. There are more, such as hardcoded /usr/lib, use of %attr for ordinary +x (prefer %install and fixing the upstream install code), missing ldconfig scriptlets, missing licensing files, incorrect license tag, license clarification needed. Just to mention a few.


Please keep the "Spec URL:" and "SRPM URL:" lines in this ticket up-to-date, then run "fedora-review -b 1052060" for some helpful reviewing checks.

(In reply to guru2018 from comment #8)
> Hello,
>  I am one of developer involved in the development of ip2location library.

Where is Chris Lim?

Who will be the final submitter(packager)?

Guru2018 and I are from the same team of ip2location development. guru2018 is taking over the task for the fixing and fine-tuning.

(In reply to Chris Lim from comment #14)
> Guru2018 and I are from the same team of ip2location development. guru2018
> is taking over the task for the fixing and fine-tuning.

Understand now.

But here are my advices:

1. One being as the submitter should continue the work.

2. If the original submitter has some reasons that block him from continuing the job, let the people coming after create a new bug and mark this as duplicate.

3. Follow:

https://fedoraproject.org/wiki/Join_the_package_collection_maintainers

For people coming from one team/company, independent acccounts are needed, please don't use your teammate's account to update the package.

Now I'm concerned about the new SRPM. ;)

Thanks.

Hello Michael Schwendt,
 Yes, we should create 2 RPM for this. I am working on it and rest of your comments.  

 New bug is reported, and this is marked as clone. 
 new bug, 
 https://bugzilla.redhat.com/show_bug.cgi?id=1081434

Guru

*** Bug 1081434 has been marked as a duplicate of this bug. ***

*** This bug has been marked as a duplicate of bug 1081434 ***