Bug#839289: RFS: pnmixer/0.7-1 [ITP] -- Simple mixer application for system tray
On Sat, Oct 1, 2016 at 2:23 PM, Arnaud Rébillout wrote:
> I am looking for a sponsor for my package "pnmixer"
I don't intend to sponsor this but here is a review:
These issues block the upload:
> pnmixer (0.7-1) UNRELEASED; urgency=medium
You should list the suite (usually unstable) where you intend the
upload to be added to instead of UNRELEASED.
The file downloaded by uscan is different to the one you have included
in your Debian source package. Please adjust your debian/watch file to
use the correct one.
These issues would be nice to fix:
I suggest that you try and make the github generated tarball as
similar as possible to the `make dist` generated tarball. You can use
`diffoscope` to compare the two tarballs.
The debian/watch tells uscan that v0.7-rc1 is newer than v0.7. I think
you want uversionmangle=s/-rc/~rc/
You may want to sign your upstream tarballs, git tags and git commits:
https://wiki.debian.org/debian/watch#Cryptographic_signature_verification
https://mikegerwitz.com/papers/git-horror-story
The images say they were produced in GIMP and look like multi-layer
images that have been rendered to flat bitmaps, did you discard the
XCF images or are they hidden away somewhere? How were these images
produced? In general it is best to render final images from the source
material at build time, using xcf2png or similar.
The bug #816124 should have be tagged fixed-upstream when you commented:
https://www.debian.org/Bugs/server-control#tag
Generated and stamp files should be removed from the upstream git repo
and added to .gitignore:
stamp-h.in
I expect most of autogen.sh can be replaced with a call to autoreconf:
autoreconf --force --install --symlink --warnings=all
Running wrap-and-sort would make diffs of the Debian packaging easier to read:
wrap-and-sort --short-indent --wrap-always --sort-binary-packages
--trailing-comma
The system() and g_spawn_command_line_async() functions should not be
used and I don't think `which` is very portable. Instead of
system()+`which` you should use g_find_program_in_path(). Instead of
g_spawn_command_line_async() you should use g_spawn_async().
http://bonedaddy.net/pabs3/log/2014/02/17/pid-preservation-society/
Please add some upstream metadata:
https://wiki.debian.org/UpstreamMetadata
Since you are upstream, please read our guide for upstreams:
https://wiki.debian.org/UpstreamGuide
Automatic checks:
build:
ui-tray-icon.c:340:2: warning: ‘gtk_status_icon_set_from_pixbuf’ is
deprecated [-Wdeprecated-declarations]
ui-tray-icon.c:358:2: warning: ‘gtk_status_icon_set_tooltip_text’ is
deprecated [-Wdeprecated-declarations]
ui-tray-icon.c:400:2: warning: ‘gtk_status_icon_position_menu’ is
deprecated [-Wdeprecated-declarations]
ui-tray-icon.c:578:2: warning: ‘gtk_status_icon_new’ is deprecated
[-Wdeprecated-declarations]
ui-tray-icon.c:604:2: warning: ‘gtk_status_icon_set_visible’ is
deprecated [-Wdeprecated-declarations]
lintian:
P: pnmixer source: debian-watch-may-check-gpg-signature
check-all-the-things:
$ env PERL5OPT=-m-lib=. duck
E: debian/control: Vcs-Git:
https://anonscm.debian.org/collab-maint/pnmixer.git: ERROR
(Certainty:certain)
fatal: repository
'https://anonscm.debian.org/collab-maint/pnmixer.git/' not found
E: debian/control: Vcs-Browser:
https://anonscm.debian.org/gitweb/?p=collab-maint/pnmixer.git;a=summary:
ERROR (Certainty:certain)
Curl:0 HTTP:404 No error
$ env PERL5OPT=-m-lib=. cme check dpkg
...
Warning in 'control source Vcs-Browser' value
'https://anonscm.debian.org/gitweb/?p=collab-maint/pnmixer.git;a=summary':
URL to debian system is not the recommended one
Warning in 'control source Vcs-Git' value
'https://anonscm.debian.org/collab-maint/pnmixer.git': URL to debian
system is not the recommended one
# This command checks style. While a consistent style
# is a good idea, people who have different style
# preferences will want to ignore some of the output.
$ find -type f \( -iname '*.sh' -o -iname '*.bash' \) -exec bashate
--ignore E002,E003 {} +
E001: Trailing Whitespace: ' echo '
- ./autogen.sh : L25
E010: Do not on same line as for: 'for coin in `find $srcdir -name
configure.ac -print`'
- ./autogen.sh : L96
E001: Trailing Whitespace: 'do '
- ./autogen.sh : L97
E001: Trailing Whitespace: ' if test -z "$NO_LIBTOOLIZE" ; then '
- ./autogen.sh : L124
4 bashate error(s) found
# Check with upstream where the GIMP XCF source files are.
$ find -type f \( -iname '*.png' -o -iname '*.gif' -o -iname '*.jpg'
-o -iname '*.jpeg' \) -exec grep -iF gimp {} +
Binary file ./data/icons/pnmixer.png matches
Binary file ./data/pixmaps/pnmixer-high.png matches
Binary file ./data/pixmaps/pnmixer-muted.png matches
Binary file ./data/pixmaps/pnmixer-low.png matches
Binary file ./data/pixmaps/pnmixer-medium.png matches
Binary file ./data/pixmaps/pnmixer-about.png matches
$ find -type f -iname '*.sh' -exec checkbashisms {} +
could not find any possible bashisms in bash script ./.travis/script.sh
could not find any possible bashisms in bash script ./.travis/before_install.sh
could not find any possible bashisms in bash script ./.travis/install.sh
$ codespell --quiet-level=3
./README.md:38: Continous ==> Continuous
./ChangeLog:70: Continous ==> Continuous
./ChangeLog:79: scoll ==> scroll
./src/ui-prefs-dialog.c:488: positon ==> position
./src/Doxyfile:1710: managable ==> manageable, manageably
$ fdupes -q -r . | grep -vE
'/(\.(git|svn|bzr|hg|sgdrawer|pc)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s
./data/icons/pnmixer.png
./data/pixmaps/pnmixer-about.png
$ flawfinder -Q -c .
<lots>
$ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec POFileChecker {} +
<lots>
$ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec POFileSpell {} +
<lots>
# check if these can be switched to https://
$ grep -rF http: .
<lots>
$ find -type f \( -iname '*.po' -o -iname '*.pot' -o -iname '*.mo' -o
-iname '*.gmo' \) -exec i18nspector --jobs 1 {} +
I: ./po/zh_CN.po: boilerplate-in-initial-comments "Copyright (C) YEAR
THE PACKAGE'S COPYRIGHT HOLDER"
W: ./po/zh_CN.po: invalid-date PO-Revision-Date: (empty string)
W: ./po/zh_CN.po: no-report-msgid-bugs-to-header-field
W: ./po/fr.po: no-report-msgid-bugs-to-header-field
I: ./po/ru.po: boilerplate-in-initial-comments "Copyright (C) YEAR THE
PACKAGE'S COPYRIGHT HOLDER"
W: ./po/ru.po: no-report-msgid-bugs-to-header-field
W: ./po/ru.po: invalid-last-translator 'Pavel Roschin <rpg89(at)post(dot)ru>'
W: ./po/it.po: no-report-msgid-bugs-to-header-field
W: ./po/nl.po: no-report-msgid-bugs-to-header-field
W: ./po/hr.po: no-report-msgid-bugs-to-header-field
W: ./po/vi.po: no-report-msgid-bugs-to-header-field
W: ./po/de.po: no-report-msgid-bugs-to-header-field
W: ./po/uk.po: no-report-msgid-bugs-to-header-field
$ find -type f \( -iname '*.c' -o -iname '*.cc' -o -iname '*.cxx' -o
-iname '*.cpp' -o -iname '*.h' -o -iname '*.hh' -o -iname '*.hxx' -o
-iname '*.hpp' \) -exec include-what-you-use {} \;
<lots>
$ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec msgfmt
--check --check-compatibility --check-accelerators
--output-file=/dev/null {} \;
<lots>
$ find -type f \( -iname '*.sh' -o -iname '*.bash' -o -iname '*.zsh'
\) -exec shellcheck {} +
<lots>
$ find -type d \( -iname .bzr -o -iname .git -o -iname .hg -o -iname
.svn -o -iname CVS -o -iname RCS -o -iname SCCS -o -iname _MTN -o
-iname _darcs -o -iname .pc -o -iname .cabal-sandbox -o -iname .cdv -o
-iname .metadata -o -iname CMakeFiles -o -iname _build -o -iname
_sgbak -o -iname autom4te.cache -o -iname blib -o -iname cover_db -o
-iname node_modules -o -iname '~.dep' -o -iname '~.dot' -o -iname
'~.nib' -o -iname '~.plst' \) -prune -o -type f ! \( -iname '*.bak' -o
-iname '*.swp' -o -iname '#.*' -o -iname '#*#' -o -iname 'core.*' -o
-iname '*~' -o -iname '*.gif' -o -iname '*.jpg' -o -iname '*.jpeg' -o
-iname '*.png' -o -iname '*.min.js' -o -iname '*.js.map' -o -iname
'*.js.min' -o -iname '*.min.css' -o -iname '*.css.map' -o -iname
'*.css.min' -o -iname '*.wav' \) -exec env PERL5OPT=-m-lib=.
spellintian --picky {} +
<lots>
$ grep -riE 'fixme|todo|hack|xxx+|broken' .
<several>
--
bye,
pabs
https://wiki.debian.org/PaulWise
Reply to: