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

Re: Bug#800406: RFS sayonara/0.8.2



Hi Lucio,


>Those libraries are loaded on runtime. The program will start without them, 
>but it won't work as intended. They are not available because the build 
>process does not depend on them. 


wonderful.

But you might drop many libraries from build-depends
qt5-default, qttools5-dev, qttools5-dev-tools, libqt5sql5-sqlite,


stuff such as
libqt5core5a, libqt5dbus5, libqt5network5, libqt5sql5, libqt5xml5 gets picked up
by the development libraries, avoding to have SONAME issues on transitions.


let me know if you have questions about the issues, leaving Ross to help you now :)

btw wrap-and-sort please :)
(you just need to run the command)
and between Description: and the start there should be a space
(I downloaded the package from the ppa, not sure if the git is different)


Gianfranco

2016-04-05 21:50 GMT+02:00 Gianfranco Costamagna <costamagnagianfranco@yahoo.it>:

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: