LMS-Community / plugin-Qobuz

Squeebox plugin to stream from qobuz.com
30 stars 16 forks source link

Useless code line 529 in plugin.pm #68

Closed Sveninndh closed 8 months ago

Sveninndh commented 8 months ago

Useless code line 529 in plugin.pm

sub browseArtistMenu my ($client, $cb, $params, $args) = @_;

my $items = []; # this line is not needed
michaelherger commented 8 months ago

Thanks for that! I'm sure there's more...

Sveninndh commented 8 months ago

The 'extra' parameter in function getGenre() is deprecated since a long time.

This means the getGenre() function in api.pm will never get a value for subgenresCount or albums

`sub getGenre { my ($self, $cb, $genreId) = @_;

$self->_get('genre/get', $cb, {
    genre_id => $genreId,
    extra => 'subgenresCount,albums',
    _ttl  => QOBUZ_EDITORIAL_EXPIRY,
});

}`

This also means that all the code in QobuzGenre() in plugin.pm that evaluates these values is pointless, as is the call to getGenre() itself.

You can therefore remove getGenre() in api.pm and shorten QobuzGenre() as follows in plugin.pm without changing the functionality.

`sub QobuzGenre { my ($client, $cb, $params, $args) = @_;

my $genreId = $args->{genreId} || '';

my $items = [{
    name => cstring($client, 'PLUGIN_QOBUZ_BESTSELLERS'),
    url  => \&QobuzFeaturedAlbums,
    image => 'html/images/albums.png',
    passthrough => [{
        genreId => $genreId,
        type    => 'best-sellers',
    }]
},{
    name => cstring($client, 'PLUGIN_QOBUZ_NEW_RELEASES'),
    url  => \&QobuzFeaturedAlbums,
    image => 'html/images/albums.png',
    passthrough => [{
        genreId => $genreId,
        type    => 'new-releases',
    }]
},{
    name => cstring($client, 'PLUGIN_QOBUZ_PRESS'),
    url  => \&QobuzFeaturedAlbums,
    image => 'html/images/albums.png',
    passthrough => [{
        genreId => $genreId,
        type    => 'press-awards',
    }]
},{
    name => cstring($client, 'PLUGIN_QOBUZ_EDITOR_PICKS'),
    url  => \&QobuzFeaturedAlbums,
    image => 'html/images/albums.png',
    passthrough => [{
        genreId => $genreId,
        type    => 'editor-picks',
    }]
},{
    name => cstring($client, 'PLUGIN_QOBUZ_PUBLICPLAYLISTS'),
    url  => \&QobuzPublicPlaylists,
    image => 'html/images/playlists.png',
    passthrough => [{
        genreId => $genreId,
        type    => 'editor-picks',
    }]
},{
    name => cstring($client, 'PLUGIN_QOBUZ_LATESTPLAYLISTS'),
    url  => \&QobuzPublicPlaylists,
    image => 'html/images/playlists.png',
    passthrough => [{
        genreId => $genreId,
        type    => 'last-created',
    }]
}];

$cb->({
    items => $items
});

}`

michaelherger commented 8 months ago

Please create PRs if you have larger changes to discuss. It's much harder to understand from a textual description what you'd like me to change.

(and you probably shouldn't share documentation - I've been asked to take down a repository with Qobuz documentation before...)

Sveninndh commented 8 months ago

Danke für den Hinweis.

Der Einfachheit halber kurz auf Deutsch.

Die vorgeschlagene Änderung ist sehr einfach.

1.) Löschen nicht mehr verwendetet Funktion getGenre() in api.pm 2.) Anpassen der Funktion QobuzGenre() dem entsprechend oder einfach mit dem Kode oben überschreiben.

Noch eine kurze Bitte. Du hast ja 'My weekly Q' jetzt eingebaut und einen entsprechenden Hinweis in GitHub

'The Weekly Q. Let's enjoy it while it lasts... Thanks @Sveninndh!'

dazugeschrieben.

Es wäre ganz schön wenn man in solchen fällen zusätzlich noch einen Einzeiler in die plugin.pm schreiben könnte.

2024-01-21 The Weekly Q. Let's enjoy it while it lasts... Thanks @Sveninndh!

Der Hinweis in Github verschwindet nach einiger Zeit aus dem Focus und dann weis das kein Mensch mehr. Das gilt natürlich auch für Andere die tollen Kode beitragen soweit es nicht Marginales oder irgendwelche Fixes sind.

VG

michaelherger commented 8 months ago

Das ist eben noch einmal ein guter Grund um Pull Requests zu erstellen: du erscheinst dann automatisch als Contributor (https://github.com/pierrepoulpe/SqueezeboxQobuz/graphs/contributors) - und auch in deinem Profil!

Wenn ich dich jetzt im Code erwähne, dann muss ich das für alle anderen auch nachholen. Mit einem PR wäre das schon erledigt. Kannst ja als ersten PR gleich alle die Namen in Plugin.pm eintragen? Damit wärst du sowohl auf Github als auch im Code verewigt?

darrell-k commented 8 months ago

You really should give git and PRs a go. It's pretty straightforward to get started and there's lots of howtos on the internet. It makes it far easier to incorporate tested changes error-free into the main repo.

We should always respect the workflows set up for various open source projects. Otherwise it quickly becomes onerous on people like Michael.

I mentioned a little while ago that perhaps we should have a changelog viewable from the plugin settings page, to make users aware of new features, and maybe even find some way of notifying them so they can take a look after updating the plugin. It could also contain credits.

I don't think we should clutter up the main body of the code with such things.

Sveninndh commented 8 months ago

ok, ok, lasst mir etwas Zeit, ich werde mir github mal etwas genauer anschauen.

SamInPgh commented 8 months ago

@Sveninndh Here's a link to an article Michael shared with me when I was getting started with Github. I found it very useful. I hope you will too. Welcome to the club!

https://opensource.com/article/19/7/create-pull-request-github