[Bug 988667] Review Request: valadoc - Documentation tool for the vala project

[bugzilla at redhat.com]

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



--- Comment #21 from Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl> ---
(In reply to Gergely Polonkai from comment #17)
> Created attachment 1142088 [details]
> .spec file for valadoc 0.30
> 
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #16)
> > In order of importance:
> > - Source0:
> > https://git.gnome.org/browse/valadoc/snapshot/valadoc-valac-%{version}.tar.xz

Version appears to be 0.30, not 0.30.0 (so the tarball URL works).

> > - %{_libdir}/libvaladoc.so should be in -devel, not in the main package
> 
> No it shouldn’t. ldd shows that the valadoc binary uses libvaladoc.so

It doesn't here :)

$ ldd
/home/zbyszek/rpmbuild/BUILDROOT/valadoc-0.30-1.fc23.x86_64/usr/bin/valadoc
        linux-vdso.so.1 (0x00007ffdc9988000)
        libvaladoc.so.0 => ...

https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages has an
explanation.

> > - %description should be extended a bit to say something that valadoc
> > extracts documentation from the source code (or whatever, I'm just guessing
> > here).
> 
> I have added some more text, but I’m not really good at this. I will look
> into this deeper soon.

I think current %description is OK.

> > - %{buildroot} is preferred to $RPM_BUILD_ROOT and it's considered bad style
> > to use both
> 
> I have fixed that. I also added a / between %{buildroot} and %{_libdir} for
> the sake of readability, but I’m not sure if it should be there

It's not necessary, and it will cause a double slash in the paths shown in
build logs, but some people like to add it. Either way is fine.

> > - You can use %make_build instead of make %{?_smp_mflags} and %make_install
> > instead of make install DESTDIR=%{buildroot}. More consise.
> 
> I fixed that. I hope I got it right

Looks correct. But the package doesn't build here, rpm fails to find
%{_libdir}/valadoc/drivers/0.30.x/libdriver.so

(In reply to Raphael Groner from comment #20)
> . Please use %{_mandir} .
> %{_mandir}/man1/%{name}.1.gz
Or even %{_mandir}/man1/%{name}.1.*
in case the compression ever changes.

> . Generally, you can use %{name} everywhere in the spec file, where
> 'valadoc' is set instead.
Please don't :)
People do that, but there's no requirement (or suggestion) in the guidelines to
do that, and it makes everything *much* harder to read. Using macros make sense
for things which vary between builds or change over time. The name of the
package is something that almost never changes.

-- 
You are receiving this mail because:
You are always notified about changes to this product and component