Thanks for the reply and help getting this tuned up.

I've addressed each notation, point by point, below:

On Wed, Jan 22, 2014 at 8:37 AM, Benjamin Bertrand <
bee...@xboxmediacenter.com> wrote:

> 1. This is a script, not a plugin. Would it be ok to rename it
> "script.audio.pandora" or "script.pandora" for consistency?
>

I'm not attached to the naming here; "script.audio.pandora" would be fine.
But I think the current name might be more correct. I've read the "Add-on
Structure" document, specifically the Directory Name section @
http://wiki.xbmc.org/index.php?title=Add-on_structure#Directory_Name, and
looked at the LastFM plugin ( @
https://github.com/XBMC-Addons/plugin.audio.lastfm ). I've used these as
references, and I'd like this to be "A music add-on that will appear in the
Music main menu", not to appear in the Program menu.

Am I misreading this?

After looking at the addon.xml again, I noticed that the
"xbmc.python.script" was used as an extension point. I've changed it to
"xbmc.python.pluginsource". Does that correct this issue? And do you think
that is the correct designation for the addon?

Again... not attached to the name or extension point/type... I want it to
be correct and consistent.


> 2. Remove the Thumbs.db files:
> addonpr.addonparser - ERROR   -
> plugin.audio.pandora/resources/skins/Alaska/media/Thumbs.db is not
> allowed
> addonpr.addonparser - ERROR   -
> plugin.audio.pandora/resources/skins/Smoke/media/Thumbs.db is not
> allowed
>

Done.

3. Please add the language tag. See
> http://wiki.xbmc.org/index.php?title=Addon.xml#.3Clanguage.3E
>

Done, added to addon.xml. Does it need to be anywhere else?


> 4. I saw you know setting PLAYER_CORE_* is deprecated, but it seems
> you need it, so it's ok for now.
>

As you have likely seen, I have engaged in discussion about this (
http://forum.xbmc.org/showthread.php?tid=173887&pid=1603435#pid1603435 )
and added a bug report as well ( http://trac.xbmc.org/ticket/14854 ).

I'll be happy to remove it once there is an accepted alternative available.


> 5. Please add xml header with proper encoding to all your xml files:
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/settings.xml
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/skins/Alaska/skin.xml
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/skins/Alaska/720p/script-pandora.xml
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/skins/Alaska/NTSC/script-pandora.xml
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/skins/Default/skin.xml
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/skins/Default/720p/script-pandora.xml
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/skins/Default/NTSC/script-pandora.xml
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/skins/Smoke/skin.xml
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/skins/Smoke/720p/script-pandora.xml
> addonpr.addonparser - ERROR   - No xml encoding specified in
> plugin.audio.pandora/resources/skins/Smoke/NTSC/script-pandora.xml
>

Done.


> 6. If possible, try to replace print statements with xbmc.log()
>

Done.


> On Wed, Jan 22, 2014 at 1:21 AM, Roy Ivy III <rivy....@gmail.com> wrote:
> > * addon -   plugin.audio.pandora
> > * version - 2.3.1
> > * url - git://github.com/rivy/xbmc-plugin.audio.pandora.git
> > * revision - 011e214
> > * branch - master
> > * tag - R-v2.3.1
> > * xbmc version - frodo + gotham
> >
> > Pandora Radio for XBMC
> > + adds the ability to listen to Pandora audio streams
> > + mature, in-use / development for almost 4 years
> >
> > Thanks for the consideration.
> > --
> > Roy
>

Thanks for taking the time to review the addon.

I'll test these changes and then post another [Pull Git] request with the
most recent update later today.
------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Xbmc-addons mailing list
Xbmc-addons@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xbmc-addons

Reply via email to