Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update hooks for TvTunes #4826

Merged
merged 1 commit into from Jun 6, 2014
Merged

Update hooks for TvTunes #4826

merged 1 commit into from Jun 6, 2014

Conversation

robwebset
Copy link
Contributor

I hope it's OK to create this - didn't know if I should or not!

Just a small change to the confluence hooks, will do a couple of things

  • Prevents some errors with video paths with unusual characters in
  • Adds support for music videos

Would also be good to get this in the Gotham branch for the next version of Gotham

Thanks

Rob

@MartijnKaijser
Copy link
Member

i rather have it removed completely

@capfuturo
Copy link

That would be insulting, destructive and deconstructive towards the hard work others have previously put into this add-on so it could make it to Confluence stock in first place, which is 'enjoyed by many and hated by fewers'.

@MartijnKaijser
Copy link
Member

I can certainly live with that feeling

@jmarshallnz
Copy link
Contributor

It's not up to @MartijnKaijser, it's up to the skin maintainers @ronie, @BigNoid, @Black09.

Seems to me the options are fix or remove. Given there's been no particular protests regarding it being in place, I don't see why it needs removing.

@Black09
Copy link
Member

Black09 commented May 31, 2014

I'm also not a fan of tv tunes but no one is forced to use it, so no need to remove it. :)

Change looks ok to me.

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone May 31, 2014
@kib
Copy link
Member

kib commented Jun 5, 2014

Change looks good to me as well. TvTunes is an optional include in Confluence anyway.

@MartijnKaijser
Copy link
Member

i'm fine with at least fixing things that are currently included. adding new things is another discussion though.

MartijnKaijser added a commit that referenced this pull request Jun 6, 2014
@MartijnKaijser MartijnKaijser merged commit ddf5a4a into xbmc:master Jun 6, 2014
@MartijnKaijser MartijnKaijser modified the milestones: Helix 14.0-alpha1, Pending for inclusion Jun 6, 2014
@t-nelson t-nelson removed the Gotham label Jun 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants