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



--- Comment #2 from Sander Hoentjen <[email protected]> ---
(In reply to Roman Tsisyk from comment #1)
> Thanks your for you spec!
Thanks for reviewing!
> 
> > #%license COPYING
> 
> License is mandatory for all new packages [1].
> Your ticket has been fixed in the upstream, please enable
> https://github.com/PowerDNS/pdns/pull/3202
> 
> [1]:
> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
> 
Will fix this with the release of second alpha, expected soon
> --------------
> 
> > BuildRequires: systemd-units
> 
> => BuildRequires: systemd
> 
> systemd-units has been merged into systemd package [2].
> 
> [2]: https://fedoraproject.org/wiki/Packaging:Systemd#Filesystem_locations
Yes but for EPEL 6 you need systemd-units right? So that works on al releases.
If I need to make it conditional I can of course.
> 
> --------------
> 
> > # install systemd unit file
> > %{__install} -D -p -m 644 contrib/%{name}.service 
> > %{buildroot}%{_unitdir}/%{name}.service
> 
> + Requires(post): systemd
> + Requires(preun): systemd
> + Requires(postun): systemd
> +
> + %post
> + %systemd_post %{name}.service
> +
> + %preun
> + %systemd_preun %{name}.service
> +
> + %postun
> + %systemd_postun_with_restart %{name}.service
> 
> [3] https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#New_Packages
> 
Will fix
> --------------
> 
> > %{_mandir}/man1/%{name}.1.gz
> 
> > When installing man pages, note that they should be installed uncompressed 
> > as the build system will compress them as needed. The compression method 
> > may change, so it is important to reference the pages in the %files section 
> > with a pattern that takes this into account [4]
> 
> [4]: https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages
> 
> >  /usr/bin/install -c -m 644 dnsdist.1 
> > '/builddir/build/BUILDROOT/dnsdist-1.0.0-0.1.alpha1.fc24.x86_64/usr/share/man/man1'
> 
Ok, will fix
> --------------
> 
> dnsdist-1.0.0-alpha1/html/js/jquery-1.8.3.min.js
> dnsdist-1.0.0-alpha1/html/js/jsrender.js
> dnsdist-1.0.0-alpha1/html/js/moment.min.js
> dnsdist-1.0.0-alpha1/html/js/purl.js
> dnsdist-1.0.0-alpha1/html/js/rickshaw.min.js
> dnsdist-1.0.0-alpha1/html/js/underscore-min.js
> ...
> 
> Obfuscated (=compiled) JavaScript looks like a binary for me:
> 
> > When you encounter prebuilt binaries in a package you MUST:
> >
> > Remove all pre-built program binaries and program libraries in %prep prior 
> > to the building of the package.
> > Ask upstream to remove the binaries in their next release. [5]
> 
> [5]
> https://fedoraproject.org/wiki/Packaging:Guidelines#No_inclusion_of_pre-
> built_binaries_or_libraries
> 
> I'm not so experienced with Fedora packaging yet, but some other distros
> (you know) blame for all these *.min.js VERY strongly, so upstream might
> have problems with other packages as well. Please correct me if I'm wrong.

I will look into this
> 
> --------------
> 
> %dir %{_sysconfdir}/%{name}/
> 
> It would be nice to include an example of configuration file.
> 
Ok, will do that
> --------------
> 
> Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
> dnsdist-debuginfo
> 
> --------------
> 
> > -I/builddir/build/BUILD/dnsdist-1.0.0-alpha1/ext/mbedtls/include
> 
> mbedtls.i686 : Light-weight cryptographic and SSL/TLS library
> mbedtls.x86_64 : Light-weight cryptographic and SSL/TLS library
> mbedtls-utils.x86_64 : Utilities for mbedtls
> mbedtls-static.i686 : Static files for mbedtls
> mbedtls-static.x86_64 : Static files for mbedtls
> mbedtls-devel.i686 : Development files for mbedtls
> mbedtls-devel.x86_64 : Development files for mbedtls
> mbedtls-doc.noarch : Documentation files for mbedtls
> 
ok, will look into unbundling
> --------------
> 
> ^I--disable-silent-rules \$
>   --enable-dnscrypt \$
>   --enable-libsodium \$
> ^I--with-lua \$
> 
> (`:set list` in vim)
Good catch
> --------------
> 
> > Summary: A highly DNS-, DoS- and abuse-aware loadbalancer
> 
> Remove the "A" article from Summary, it looks better in listings without.
> It is not mandatory and is not mentioned in the guidelines, but I was
> noticed about that couple times by Zbigniew Jędrzejewski-Szmek.
> 
ok
> --------------
> 
> Please try to use koji to ensure that package compiles on all supported
> architectures:
> koji build --scratch rawhide pdns-4.0.0-0.1.alpha1.fc24.src.rpm
> 
will do
> --------------
> 
> What the difference between https://github.com/PowerDNS/pdns and this
> software?
> Shouldn't dnsdist to be added as a subpackage of existing pdns package?
> 
dnsdist lives in the pdns tree, but it is a fully seperate thing.
https://www.powerdns.com/nluug/2015%20nluug%20powerdns%20dnsdist.pdf
"Open source, vendor neutral - it is not “PowerDNS Dist”"
> --------------
> 
> It seems that pdns packages also uses yahttp.
> Probably yahttp needs their own package.
Will look into this
> 
> --------------
> 
> License and unclear relation with existing pdns package are major problems
> here.
Hope I have resolved these, will try to have a new spec ready soon.

-- 
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
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to