AudioTube in KDEReview

Harald Sitter sitter at kde.org
Wed Jun 16 13:39:20 BST 2021


Hola

On 16.06.21 00:44, Jonah BrĂ¼chert wrote:
> I would like to move AudioTube to KDEReview.

- the source is almost reuse compliant but not quite. needs some extra
data files equipped with CC-0 or the like [1][2]. You could then also
add an include on
https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-reuse.yml
to your gitlab-ci.yml to ensure it stays at complete coverage

- l10n is not correctly set up. You need to set a translation domain on
the KLocalizedContext

- app crashes when ytmusicapi isn't installed ;) maybe add a
find_package that simply tries to run a simple script importing it? tag
the package RUNTIME. that way the runtime dep is properly codified

- same for youtube_dl which seems to be required behind the scenes

- app isn't kcrash enabled :((

- seeking doesn't seem to do anything. I get `audiotube(103398)/(qml)
onMoved: Value: 71454.30987821381` as output but then the seek doesn't
actually happen

- something is also wonky with the playlist. it doesn't cover the width
(when data fails to load). it is overlapped by the scrollbar. clear and
shuffle don't seem to have labels though there would be enough space
https://i.imgur.com/Q3IoaBj.png

- on some artists I get `TypeError: 'NoneType' object is not iterable `
https://i.imgur.com/U8BKHbQ.png

- on the subject of errors. I'm not sure the passive popup overlapping
the controls at the bottom is all that nice

- appdata should probably describe the <launchable>

- ytmusic.cpp forces LC_ALL to en_us, that seems a bit fishy and it
doesn't explain why either

- VideoInfoExtractor ctor should be explicit

- PlaylistUtils looks like it maybe should be a namespace not a class.
feel free to ignore me if you disagree

- in asyncytmusic.h there's a typo in 'ininitalized'

- ~YTMusicThread be tagged `override`

- example has a type in 'excaustive'

- you may want to do a pass over all strings. I'm seeing some action
strings not using title capitalization (e.g. i18n("Add to playlist")
i18n("Play next")) [3]

[1] https://apachelog.wordpress.com/2021/04/06/reuse-licensing-helper/
[2]
https://community.kde.org/Guidelines_and_HOWTOs/Licensing#License_Statements_in_Non-Source-Code_Files
[3] https://develop.kde.org/hig/style/writing/capitalization/


More information about the kde-core-devel mailing list