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

Bug#783225: RFS: corebird/1.0-1 [ITP] -- Modern, easy and fun Twitter client



On Fri, Apr 24, 2015 at 4:32 PM, Edward Betts wrote:

> corebird   - Modern, easy and fun Twitter client

I don't intend to sponsor this but here is a review:

Harlan's review is good, please take a look at it.

examples/media*.jpg do not look like they were created by or are owned
by upstream, so they are probably illegal to redistribute. Please
remove the package from mentors until this is fixed.

autotools-dev almost always isn't needed when you have dh-autoreconf.

NOCONFIGURE has to be set when calling autogen.sh, otherwise the it
will run ./configure, but that should only happen from
dh_auto_configure.

Many of the comments in debian/rules are not needed.

override_dh_installman is not needed as the upstream build system
installs the manual page.

Why do you disable the upstream test suite? That is usually not a good idea.

I would suggest wrapping debian/menu at whitespace.

I would suggest adding a longtitle and icon to debian/menu, probably
the one from the upstream .desktop file.

https://www.debian.org/doc/manuals/maint-guide/dother.en.html#menu

The upstream README.md contains dependency, distro and compiling info.
None of this is useful to people installing binary packages, you might
want to strip it out during the build process using sed.

You might want to run `wrap-and-sort -sa` to wrap debian/control and
other files so debdiffs are more readable.

debian/clean shouldn't be needed, unless the upstream build system
isn't cleaning up properly.

I'd suggest that upstream should remove the PNG files from git and
render them from the SVG files at build time.

vapi/gdk-pixbuf-2.0.vapi is a generated file but I can't find the
source code for it. I'd suggest upstream remove it from git and
generate it at build time instead.

Automated checks:

https://lintian.debian.org/
https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git

$ lintian
P: corebird source: debian-watch-may-check-gpg-signature
<may be more if the package was buildable>

$ bashate --ignore E002
E010: Do not on same line as for: 'for size in ${sizes[@]}'
 - ./assets/render-icons.sh: L7
E003: Indent not multiple of 4: '  rsvg-convert ./corebird.svg
--width="${size}" --height="${size}" \'
 - ./assets/render-icons.sh: L9
E003: Indent not multiple of 4: '               --format=png -o
"./${size}x${size}/corebird.png"'
 - ./assets/render-icons.sh: L10
3 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 ./assets/no_banner.png matches

$ codespell --quiet-level=3
./README.md:28: distrubution  ==> distribution
./data/org.baedert.corebird.appdata.xml.in:12: abilty  ==> ability

$ fdupes -q -r .
./tests/__sql_init1.sql
./tests/_sql_init1.sql

$ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec POFileChecker {} +
<lots of output>

$ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec POFileSpell {} +
<lots of output>

$ find -type f \( -iname '*.po' -o -iname '*.pot' -o -iname '*.mo' -o
-iname '*.gmo' \) -exec i18nspector {} +
<lots of output>

$ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec msgfmt
--check --check-compatibility --check-accelerators
--output-file=/dev/null {} \;
<lots of output>

$ find -type f -iname '*.sh' -exec shellcheck {} +

In ./assets/render-icons.sh line 7:
for size in ${sizes[@]}
            ^-- SC2068: Double quote array expansions, otherwise
they're like $* and break on spaces.


In ./autogen.sh line 3:
srcdir=`dirname $0`
       ^-- SC2006: Use $(..) instead of deprecated `..`
                ^-- SC2086: Double quote to prevent globbing and word splitting.


In ./autogen.sh line 6:
ORIGDIR=`pwd`
        ^-- SC2006: Use $(..) instead of deprecated `..`


In ./autogen.sh line 16:
cd $ORIGDIR || exit $?
   ^-- SC2086: Double quote to prevent globbing and word splitting.

$ grep -riE 'fixme|todo|hack|xxx' .
<lots of output>

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


Reply to: