[Bug 920751] Review Request: satyr - A set of tools to create anonymous, machine-friendly problem reports

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=920751

--- Comment #3 from Martin Milata <mmilata@xxxxxxxxxx> ---
Thank you for your feedback, Michael! I tried to address the issues and
uploaded new spec and SRPM.

Spec URL: http://mmilata.fedorapeople.org/satyr.spec
SRPM URL: http://mmilata.fedorapeople.org/satyr-0.2-1.fc20.src.rpm

(In reply to comment #2)
> * Run "rpmlint -i" on the src.rpm and all built rpms. Some of the reported
> warnings/errors may be dubious (false positives), however. Be aware of that.

rpmlint still gives me spelling-error, shared-lib-calls-exit (to be fixed later
with other error handling issues), no-documentation (for the subpackages, the
main package has documentation) and private-shared-object-provides (caused by
the python bindings).

> > Source0: https://fedorahosted.org/released/abrt/satyr-%{version}.tar.xz
> 
> Checksum of included tarball doesn't match upstream. The sources downloaded
> with "spectool -g satyr.spec" are two days older and include an older
> satyr.spec.in file.

Should not happen with the new version.

> > Requires: elfutils-libelf, elfutils-libs, binutils
> > Requires: libunwind >= 1.1
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

The requires were removed.

> > Summary: A set of tools to create anonymous, machine-friendly problem reports
> 
>   Summary: Tools to create anonymous, machine-friendly problem reports
> 
> Would be even more concise (and dropping those leading articles is good with
> regard to displaying package summaries in Anaconda and in package installer
> tools). Interestingly, the spec file included within the source tarball
> contains the following summary:
> 
>   Summary: Automatic problem management with anonymous reports

Changed, thanks for the suggestion.

> > Name: satyr
> > Group: Development/Libraries
> 
> "System Environment/Libraries" is the group for shared library base packages
> (regardless of whether it includes also tools).

Changed.

> > find %{buildroot} -regex ".*\.la$" | xargs rm -f --
> 
> Amazingly/unnecessarily complex. Basic wildcard expression should suffice,
> and "rm" doesn't need "-f" because if no files were found, xargs would not
> run "rm" at all:
> 
>   find %{buildroot} -name \*.la | xargs rm

Fixed.

> > %{_mandir}/man1/%{name}.1.gz
> 
> %{_mandir}/man1/%{name}.1*   to allow for changing/reconfigured compression
> or even no compression.

Fixed.

> > THIS MANPAGE IS NOT OUT OF DATE AND THEREFORE USELESS.
> 
> Uh? ;-)

Ehm:) I replaced the manual page with slightly saner version.

> > %doc README NEWS COPYING TODO ChangeLog
> 
> "btparser 0.18"? That's odd, because a newer btparser (0.25) is in F-19
> development, so something ought to be done about these %doc files. The name
> "satyr" does not appear anywhere in them.
> 
> 
> * The build.log contains failing searches for "pdflatex", "doxygen" and
> "dot" for building the optional API documentation files.

I cleaned up the docs a bit, they should not refer to btparser anymore.
Building the API documentation is disabled for the time being.

> > /usr/include/satyr/config.h
> 
> Caution. Exposing this file as a public API header bears a risk of leading
> to trouble (because some of definitions in it will likely conflict with a
> libsatyr based program that will use an own config.h header).
> 
> $ grep config.h *
> callgraph.h:#include "config.h"
> config.h:/* lib/config.h.  Generated from config.h.in by configure.  */
> config.h:/* lib/config.h.in.  Generated from configure.ac by autoheader.  */
> core_fingerprint.h:#include "config.h"
> disasm.h:#include "config.h"

Thanks for the tip. I removed config.h from the set of exported headers and
from the files included by the exported headers.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=tW4WkGZsrD&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]