Re: Bug#800406: RFS sayonara/0.8.2
control: owner -1 !
control: tags -1 moreinfo
Hi, lets review:
(thanks Ross for the nice feedback, I'm adding something more on top of your work)
no orig tarball, please upload to mentors or push tags
please run wrap-and-sort
std version is 3.9.7
libqt5core5a, libqt5dbus5, libqt5network5, libqt5sql5, ... <-- WRONG
you need the development packages, not the libraries
gstreamer1.0-plugins-ugly, libqt5sql5-sqlite < -- aren't them picked
up by shibs:Depends?
license.txt <-- useless please remove
debian/md5sum <-- useless and autogenerated, I think you need to better
know how to create a clean upload.
"mkdir -p build && cd build && cmake .. -DCMAKE_CXX_FLAGS="$(CXX_FLAGS)" -DCMAKE_EXE_LINKER_FLAGS="$(LDFLAGS)" -DCMAKE_SHARED_LINKER_FLAGS="${LDFLAGS}" -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_BUILD_TYPE=RelWithDebInfo"
please just call dh_auto_configure and patch the build system to automatically take care of the flags.
I think all the rules hacks can be dropped.
many copyrights are missing
./src/Components/DBus/resources/org.libmtp.mtpd.object.xml: Copyright (C) 2013 Philipp Schmidt <philschmidt@gmx.net>
./src/Components/DBus/resources/org.libmtp.mtpd.folder.xml: Copyright (C) 2013 Philip Langdale <philipl@overt.org>
./src/Components/DBus/resources/org.libmtp.mtpd.folder.xml: Copyright (C) 2013 Philipp Schmidt <philschmidt@gmx.net>
just to give a few examples
./src/GUI/Player/SearchSlider.h: * Copyright (C) 2012
./src/GUI/Player/SearchSlider.cpp: * Copyright (C) 2012
copyright and there is no person owning it? (many other places have similar issues)
licenses seems good
check-all-the-things
$ codespell --quiet-level=3
$ cppcheck -j1 --quiet -f . | grep -vF 'cppcheck: error: could not find or open any of the paths given.'
$ find -type f \( -iname '*.sh' -o -iname '*.bash' -o -iname '*.zsh' \) -exec shellcheck {} +
$ flawfinder -Q -c .
$ grep -Er '/(home|srv|opt)(\W|$)' .
I didn't check lintian and to run the package, but a lot of work seems needed there.
cheers,
Gianfranco
Il Lunedì 22 Febbraio 2016 11:20, Lucio Carreras <luciocarreras@gmail.com> ha scritto:
Hi Ross,
> I have cloned your git repo to do the review, but in the future, I
recommend that you sign up for an account on Debian Mentors, and upload your package there.
Done. I already uploaded the package, but there are some minor lintian issues with the package type (native/not native) and the source/format type.
But it was late yesterday and I will proceed today evening. http://mentors.debian.net/package/sayonara When everything is alright I will give a shout.
> I was a bit surprised to so much stuff in debian/rules,
Yes, but I want to assure that the build runs out-of-tree. So one step leads to the other. But in my opinion it's pretty clear what's going on.
Nevertheless, there were some inaccuracies in the build file.
I also revised the other files you mentioned in the debian directory.
> I recommend joining the Debian Multimedia Team eventually. Then the
Vcs fields can be finalised (surrently commented out). But lets improve
the packaging a little bit more first.
Yes, with pleasure. You will hear from me later.
Thank you,
Lucio
2016-02-21 17:21 GMT+01:00 Ross Gammon <ross@the-gammons.net>:
Hi Lucio,
>
>Sorry I did not spot this email after talking you into contacting Debian
>Mentors :-)
>
>I am not a Debian Developer, so I cannot sponsor the package, but I will
>write some comments inline below that will hopefully help you on your way.
>
>On 02/15/2016 11:30 AM, Lucio Carreras wrote:
>> Dear Debian mentors,
>>
>> I am looking for a sponsor for the package "sayonara".
>>
>> * Package name: sayonara
>> * Version: 0.8.2
>> * Author: Lucio Carreras <luciocarreras@gmail.com
>> <mailto:luciocarreras@gmail.com>>
>> * Url: http://www.sayonara-player.com
>> * License: GPL3
>> * Section: audio
>>
>> Sayonara is a clear and fast audio player with its main focus on
>> managing your music library. There are many features for library
>> organization, like a tag editor or a special genre view. It also has
>> tabbed playlists, handles webstreams and podcasts, has a stream
>> recorder, a Soundcloud plugin and much more.
>>
>>
>> There's a git repository at
>>
>> https://git.sayonara-player.com/sayonara.git
>
>I have cloned your git repo to do the review, but in the future, I
>recommend that you sign up for an account on Debian Mentors, and upload
>your package there. This has some advantages:
>1. Some sponsors are used to this workflow and prefer to take packages
>from there (at least the first time)
>2. You go through a similar process to what you will go through when you
>are eventually granted upload permisions (it is good practise for later)
>3. The web interface gives a sponsor a good indication of the health of
>the package (e.g. number of lintian warnings) without having to download it.
>
>See Section 4 of mentors.debian.net/intro-maintainers
>
>> There also exist a Ubuntu PPA
>> (https://launchpad.net/~lucioc/+archive/ubuntu/sayonara) and so there
>> also exist Debian packages (available at the download section). The
>> source code in the git repository also contains a debian directory with
>> all neccessary files. It is also tested regularly with a Debian unstable
>> chroot, if there are any warnings or errors.
>
>This is a good sign. Some more detailed comments:
>
>debian/changelog:
>- As this will be the first upload to Debian, it is best that the
>changelog has one simple entry "Initial Release (Closes: #<your ITP Bug
>Number>)". This will allow the bug to be automatically closed when the
>package is uploaded to the archive. See
>https://www.debian.org/doc/manuals/developers-reference/best-pkging-practices.html#bpp-debian-changelog
>- You have set the distribution to stable. New packages should always
>target "unstable". It will automatically migrate from there to "testing"
>(if there are no problems), and then one day be released as part of the
>next stable release.
>
>debian/control
>- I recommend joining the Debian Multimedia Team eventually. Then the
>Vcs fields can be finalised (surrently commented out). But lets improve
>the packaging a little bit more first.
>- A space between "Description:" and the short description would be nice
>- The long description should probably start with "Sayonara" rather than
>"Its" because it will not be clear if read without the short description.
>
>debian/copyright
>- The "License:" stanza should be the short name of the license (e.g.
>GPL-3+).
>- Then the complete license text should be included at the bottom.
>See https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
>
>debian/rules
>- I was a bit surprised to so much stuff in debian/rules, as most of the
>time the common build systems don't need a lot more than:
>%:
> dh $@
>It might be worth looking at tweaking your cmake build system (as you
>are also upstream) so that it is not necessary to override the clean,
>build & install targets in debian/rules. This would probably benefit
>other non Debian based Linux Distros as well. Probably not critical -
>depending on the sponsor though.
>
>You are missing a watch file. This file watches for new tarballs on your
>website, and alerts everyone when there is a new version waiting to be
>packaged. See https://wiki.debian.org/debian/watch/
>
>debian/README.Debian? This should probably be deleted. You will find a
>sponsor much faster if you promise to maintain Saynora yourself (or in a
>team). :-)
>
>debian/md5sums - I am not sure this file is needed if building the
>package happens the normal way with debhelper?
>
>debian/license.txt - a license.txt in the root of the sourcecode, and
>the summarised in debian/copyright should be enough. Please remove this
>file.
>
>The package is currently building. I will let you know later if there
>are problems with that.
>
>I hope that helps you on the way. If there is something in all that
>which is not clear, or you need more help to fix it, just ask on the list.
>
>Keep up the good work, and thanks for taking an interest in getting
>Sayonara into Debian proper.
>
>Regards,
>
>Ross
>
>
Reply to: