Bug 1559864

You don't seem to have a FAS account?

(In reply to Robert-André Mauchin from comment #1)
> You don't seem to have a FAS account?

redara is my FAS account.

>Release: 0%{?dist}
This should start at 1.

>Source0: gluster-collectd-%{version}-%{_release}.tar.gz
It's preferred for Source0 to be a downloadable URL.

>cp -r conf/* %{buildroot}/%{_confdir}/
>cp -r types/* %{buildroot}/usr/share/collectd/
You should aim to preserve file timestamps. Use "cp -a" or "cp -rp". 

Also, the linked copr build failed.
>+ '%{__python2}' setup.py build
>BUILDSTDERR: /var/tmp/rpm-tmp.JzTM17: line 31: fg: no job control
The "no job control" error usually means that the macro failed to expand and the shell is interpeting the "%" as job control. You may need to add a BuildRequires for "python2-rpm-macros".

Sorry for delay, I made changes to spec file , now the build is success. 

SPEC: https://github.com/gluster/gluster-collectd/blob/master/gluster-collectd.spec

SOURCE RPM: https://copr-be.cloud.fedoraproject.org/results/redara/gluster-collectd/fedora-27-x86_64/00761614-gluster-collectd/gluster-collectd-1.0.0-1.fc27.src.rpm

can anyone pick this up and do the review please.

Sachi, could you help here

(In reply to Venkata R Edara from comment #4)
> Sorry for delay, I made changes to spec file , now the build is success. 
> 
> SPEC:
> https://github.com/gluster/gluster-collectd/blob/master/gluster-collectd.spec
> 

* Instead of /usr/share use the _datadir macro.

> Requires : collectd >= 5.8.0
> Requires : collectd-python >= 5.8.0

If the fedora releases targeted has latest above 5.8.x you can ignore the version numbers.

> %{__python2} setup.py build
* py2_build macro should achieve this. Have you tried it?

Below is a comment from Ken Dreyer from another package. Since I see python2 in many places, I think it is still relevant.

Can this package work with Python 3 instead of Python 2? Maybe it would be a good idea to add a comment to the .spec file explaining why this will not work with Python 3 if that is the case. https://fedoraproject.org/wiki/FinalizingFedoraSwitchtoPython3 is coming eventually. You could conditionalize py2/py3 with "%if %{?rhel} < 8" if you want to share the same .spec file across RHEL 7 and Fedora.

(In reply to Sachidananda Urs from comment #7)
> (In reply to Venkata R Edara from comment #4)
> > Sorry for delay, I made changes to spec file , now the build is success. 
> > 
> > SPEC:
> > https://github.com/gluster/gluster-collectd/blob/master/gluster-collectd.spec
> > 
> 
> * Instead of /usr/share use the _datadir macro.
> 
> > Requires : collectd >= 5.8.0
> > Requires : collectd-python >= 5.8.0
> 
> If the fedora releases targeted has latest above 5.8.x you can ignore the
> version numbers.
> 
> > %{__python2} setup.py build
> * py2_build macro should achieve this. Have you tried it?
> 
> Below is a comment from Ken Dreyer from another package. Since I see python2
> in many places, I think it is still relevant.
> 
> Can this package work with Python 3 instead of Python 2? Maybe it would be a
> good idea to add a comment to the .spec file explaining why this will not
> work with Python 3 if that is the case.
> https://fedoraproject.org/wiki/FinalizingFedoraSwitchtoPython3 is coming
> eventually. You could conditionalize py2/py3 with "%if %{?rhel} < 8" if you
> want to share the same .spec file across RHEL 7 and Fedora.

I added comment saying why Python 2 is supported. for Python 3 we have to test many dependencies and packages. I added macro for _datadir. but for py2_build macro the build is failing in FAS site.

here is link for successful build: https://copr-be.cloud.fedoraproject.org/results/redara/gluster-collectd/fedora-27-x86_64/00772030-gluster-collectd/

specfile: https://github.com/gluster/gluster-collectd/blob/master/gluster-collectd.spec

(In reply to Venkata R Edara from comment #8)
> (In reply to Sachidananda Urs from comment #7)
> > (In reply to Venkata R Edara from comment #4)
> > > Sorry for delay, I made changes to spec file , now the build is success. 
> > > 
> > > SPEC:
> > > https://github.com/gluster/gluster-collectd/blob/master/gluster-collectd.spec
> > > 
> > 
> > * Instead of /usr/share use the _datadir macro.
> > 
> > > Requires : collectd >= 5.8.0
> > > Requires : collectd-python >= 5.8.0
> > 
> > If the fedora releases targeted has latest above 5.8.x you can ignore the
> > version numbers.
> > 

Did you get a chance to check this? For the releases you are packaging, if they
by default ship later versions the checks are redundant.

(In reply to Sachidananda Urs from comment #9)
> (In reply to Venkata R Edara from comment #8)
> > (In reply to Sachidananda Urs from comment #7)
> > > (In reply to Venkata R Edara from comment #4)
> > > > Sorry for delay, I made changes to spec file , now the build is success. 
> > > > 
> > > > SPEC:
> > > > https://github.com/gluster/gluster-collectd/blob/master/gluster-collectd.spec
> > > > 
> > > 
> > > * Instead of /usr/share use the _datadir macro.
> > > 
> > > > Requires : collectd >= 5.8.0
> > > > Requires : collectd-python >= 5.8.0
> > > 
> > > If the fedora releases targeted has latest above 5.8.x you can ignore the
> > > version numbers.
> > > 
> 
> Did you get a chance to check this? For the releases you are packaging, if
> they
> by default ship later versions the checks are redundant.

Now i removed the versions, fedora is shipping 5.8.0 only. 
Here is latest spec : https://copr-be.cloud.fedoraproject.org/results/redara/gluster-collectd/fedora-27-x86_64/00772262-gluster-collectd/gluster-collectd.spec

Here is successful build: https://copr-be.cloud.fedoraproject.org/results/redara/gluster-collectd/fedora-27-x86_64/00772262-gluster-collectd/

I will checkin spec file to git repo, once review is over.

Looks good to me.

This is an automatic action taken by review-stats script.

The submitter account doesn't exist anymore, therefore this ticket will be closed.