https://bugzilla.redhat.com/show_bug.cgi?id=1302876
--- Comment #10 from Ingvar Hagelund <ing...@linpro.no> ---
> > Release: 1.3.20160128git%{?shortcommit0}%{?dist}
>
> The "1.3" in release looks weird; you typically use only one digit unless the
> first one is a zero. E.g. "0.3.<snapshot>" for a pre-release snapshot and
> "3.<snapshot>" for a post-release snapshot.
Yeah, it should perhaps rather be version 1.5 and release 0.3.<snapshot>, but
se below.
> Also, why are you packaging a snapshot instead of a released version?
While there are few or no actual changes in the code from the 1.4 release, the
license was changed from some unclear beerware like license to the actual BSD
license after the release. This goes for the license text inside the clatd
source, and was clearly just forgotten by upstream, as the LICENSE file had
already been updated to BSD before the release. It felt wrong to change the
license through a patch, though even when fetched from upstream. I did not want
to package clatd with two different licenses.
In the spec linked below, I wrap the 1.4 release, and change the license
through a patch, noting that the author acknowledges this. I also added the
documentation fixes. (This is in effect what happens by using the snapshot.)
> > %if 0%{?fedora} > 23
> > BuildRequires: perl-podlators
> > %endif
>
> You can get rid of the conditional if you do a BuildRequires:
> /usr/bin/pod2man
Changed to BuildRequires: %{_bindir}/pod2man as Petr suggested.
> > Requires: perl(Net::IP)
> > Requires: perl(Net::DNS)
> > Requires: perl(IO::Socket::INET6)
> > Requires: perl(File::Temp)
>
> Hmm, these should be autogenerated; but only Net::IP is. Seems like the
> dependency generator ignores requires if they don't start in column zero...
* Petr Pisar
> Collective wisdom says indented "require" statements generates too many false
> positives. Therefore generators omit them. But I'm not fully convinced about
> helpfulness of the omission.
I'll just remove perl(Net::IP), and keep the rest for now.
> > %build
> > (...)
> This all should ideally be in the upstream Makefile. Perhaps you could ask
> upstream?
Yep, I'll ask upstream for this.
> > %{_sysconfdir}/%{name}.conf
>
> This one is a configuration file; please mark it with %config(noreplace).
Fixed
Updated spec and srpm:
https://ingvar.fedorapeople.org/tayga/clatd.spec
https://ingvar.fedorapeople.org/tayga/clatd-1.4-2.fc23.src.rpm
--
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@lists.fedoraproject.org
https://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org
