https://bugzilla.redhat.com/show_bug.cgi?id=1352408
--- Comment #4 from Julian Sikorski <belegdol(a)gmail.com> ---
(In reply to Igor Gnatenko from comment #3)
(In reply to Julian Sikorski from comment #2)
> (In reply to Igor Gnatenko from comment #1)
> Thank you for taking the review! Please find my feedback below:
>
> > Missing BuildRequires: gcc
>
> Has something changed? I thought gcc was part of minimal buildroot and does
> not need to be required explicitly. In any case, something is pulling it in
> as the build is working.
Something changed. Since (~ f23) it's required to put all BRs like gcc.
Thank you! Will add it.
> > > BuildRequires: intltool
> > I would add also BuildRequires: gettext
>
> Intltool pulls gettext by requiring gettext-devel, is an explicit
> requirement necessary?
No, hopefully at some point upstream will stop using intltool and we will
switch to gettext.
If/when it happens BR adjustment will be necessary anyway, so I prefer to keep
things as they are.
> > > Requires: pkg-config
> > Drop it
>
> OK
>
> > > make %{?_smp_mflags}
> > could be replaced with %make_build
>
> OK, it was not in the template created by rpmdev-newspec on f22.
rpmdev-newspec is completely outdated.
>
> > > rm -rf $RPM_BUILD_ROOT
> > Drop it
>
> Why? rpmdev-newspec still put it in as of f24
It's not needed since ~ EL6.
Didn't know that, thank you! Will change it.
> > > find $RPM_BUILD_ROOT -name '*.la' -exec rm -f
{} ';'
> > find %{buildroot}%{_libdir} -type f -name '*.la' -delete -print
>
> Why is this needed? The version I have now is what rpmdev-newspec as of f24
> enters.
Not all packages maintained well. When you use -delete, it doesn't spawn any
command, it just uses unlinkat().
Didn't know that either, thanks! Will change it too!
> > > %find_lang %{name} --all-name
> > I think --all-name is not needed, but not sure.
>
> I replaced it with %{name}-%{apiver}
>
> > > %{_datadir}/gtk-doc/html/%{name}-0.4
> > %doc %{_datadir}/gtk-doc/html/%{name}-0.4
>
> OK
> %dir %{_datadir}/gtk-doc
> %dir %{_datadir}/gtk-doc/html
you don't need to do this.
I think I do - otherwise the directory ownership guideline [2] might be
violated I think. lasem puts files in %{_datadir}/gtk-doc but does not depend
on it, so it needs to own it. %doc only owns the lasem-0.4 folder.
>
> > > $RPM_BUILD_ROOT
> > %{buildroot}
>
> I prefer the variable format and as per current guidelines [1] both are
> acceptable.
hopefully it will be deprecated at some point. it's better to use
%{buildroot}. but as you noted, it's up to you.
Let's stay with variable version then.
> >
> > also would be great to do: %global apiver 0.4 and use it in %files.
>
> OK
>
> New releases:
> Spec URL:
https://belegdol.fedorapeople.org/lasem/lasem.spec
> SRPM URL:
https://belegdol.fedorapeople.org/lasem/lasem-0.4.3-2.fc24.src.rpm
>
> [1]
>
https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.
> 7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS
Resolution: ALMOST GOOD
New releases:
Spec URL:
https://belegdol.fedorapeople.org/lasem/lasem.spec
SRPM URL:
https://belegdol.fedorapeople.org/lasem/lasem-0.4.3-3.fc24.src.rpm
[2]
https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owne...
--
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