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
