Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

changed: remove scrobbler support #1044

Merged
merged 2 commits into from

12 participants

alanwww1 (Attila) Martijn Kaijser queeup Joakim Plate jmarshallnz ronie Arne Morten Kvarving da-anda wsoltys Zeljko Ametovic Sascha Montellese arnova
Deleted user

(soon) available as a service add-on.

the radio support is still intact but also this should be moved to an add-on eventually. with this in mind, it does not make sense to change those strings (i.e. the submission is misleading), as the whole page will be removed.

alanwww1 (Attila)
Collaborator

Just one question: So do we leave the strings in the core strings.po file ?
Wouldn't it be better to just pull them out to the addon ?

It is also interesting if it is a common situation, that we are having a lot of strings in the core strings.po file which belong to addons or skins AND not used at all by xbmc core ? There is a thread ( http://forum.xbmc.org/showthread.php?tid=132639&pid=1119545#pid1119545 ) going on about removing unused strings if possible. This was already done for Confluence( 8913d21 ), but there, it was safe to do.

Thanks for the help !

Deleted user

they should definitely be moved to the add-on. but i'm not sure whether or not ronin did that so i left them in for now.

alanwww1 (Attila)
Collaborator

Ok. Thanks for the info.

And how is the general situation ? Do you think we have a lot of strings in core used by addons ? Should we leave them in there, or move them out to the addons ?

Deleted user

there are probably some that is only used by the skin, but i don't think there are a lot (unused strings are probably a bigger issue). we should of course strive to move as many as possible to the addons.

Martijn Kaijser

One way would be to won't let add-ons/skins use core strings

queeup

Don't forget get rid of weather strings. :)

alanwww1 (Attila)
Collaborator

What we could do is prepare a PR with the strings intended to be removed, along with an announcement to addon developers to ask their help to review their addons if they have some strings to add to their own addon strings which will be removed. After a month than we can pull in that PR to actually delete the strings.

There is surely 500+ or even much more unused strings out of 2200. That is really too much.

Oppinions @cptspiff , @JezzX, anyone else ?

Joakim Plate
Collaborator
jmarshallnz
Owner

Strings have to stay shared between core and add-ons at least for some items. eg it's not like we're going to remove "Movies" anytime soon. Skins for example use plenty of core strings (the majority of strings a skin uses would be core strings).

jmarshallnz
Owner

Nice work :)

alanwww1 (Attila)
Collaborator

@jmarshallnz

What we can still do is to expand the search for used strings to all official addons and skins and come up with a list like that. After that we can still narrow this list to really just kill the ones which are obviously some leftovers from old code.

What do you think ?

ps.: sorry for hijacking this PR with this :-)

jmarshallnz
Owner

@alanwww1 absolutely - if we have a list of all strings not used in core then it's relatively quick to check the skin (my guess is @Jezz_X could tell off the top of his head whether one is used in the skin or not) which would be the main user of other strings.

My main point was that we can't rule out skins in particular using core strings.

ronie
Collaborator

@cptspiff, @alanwww1 : i did include the scrobbler related strings in the addon.
these strings can be removed: 15201 15204-15212 15217-15220 15250

alanwww1 (Attila)
Collaborator

@ronie thanks for the info. Also the translations got moved ? If not, I can help out with that.

Deleted user

so.. back on topic

Martijn Kaijser

@cptspiff
This was planned for Frodo too right?

Arne Morten Kvarving

yes, we need to open the frodo repo first.

Deleted user ghost was assigned
da-anda
Collaborator

@cptspiff I know you're around and can't resist the XBMC sirens ;)
Mind bringing this baby in shape now that Frodo is out the door? (and porbably also some other older PRs of yours)

Deleted user

rebased. i also nuked last.fm radio this time since afaik ronie's add-on does that as well. i did not sanity check the list of removed strings but i think it should be fine.

projects files done by hand so that needs verification. kept them in a separate commit for convenience, we can squash em in once confirmed.

wsoltys
Collaborator

not tested but looks fine at a first glance.

Zeljko Ametovic

builds fine on OSX

ronie
Collaborator

builds and runs fine on linux.

addons that fully replace the last.fm and libre.fm functionality are ready,
so this can go in during the march merge window as far as i'm concerned.

jmarshallnz
Owner

Assigned to merge window.

ronie
Collaborator

last.fm and libre.fm addons are in the repo now.

@cptspiff the pr needs some rebase love.

ronie ronie merged commit d5cf55a into from
arnova
Collaborator

Right: "available as a add-ons" so the code is redundant...

Sascha Montellese

This was a bit too optimistic. At least 15207 and 15208 are still used in CGUIInfoManager for the network link status in the system info window. I'll double check the others as well.

Tobias Hieta tru referenced this pull request from a commit in plexinc/plex-home-theater-public
Tobias Hieta tru Don't run servercachedatabase if we playing video
This caused stuttering, most likely because it was blocking the main
thread while doing updates to the database. So this is the Q&D way of
avoiding it.

Related to #1044
ecbfa7d
Tobias Hieta tru referenced this pull request from a commit in RasPlex/plex-home-theatre
Tobias Hieta tru Don't run servercachedatabase if we playing video
This caused stuttering, most likely because it was blocking the main
thread while doing updates to the database. So this is the Q&D way of
avoiding it.

Related to #1044
6556468
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 6, 2013
  1. changed: remove scrobbler and last.fm radio support

    spiff authored
    available as a add-ons
Something went wrong with that request. Please try again.