Skip to content

Add Podcast Series & Music Video Support#333

Merged
jreklund merged 2 commits intotboothman:masterfrom
CicerBro:master
Mar 2, 2025
Merged

Add Podcast Series & Music Video Support#333
jreklund merged 2 commits intotboothman:masterfrom
CicerBro:master

Conversation

@CicerBro
Copy link
Copy Markdown
Contributor

It should check for all valid IMDB title types because if the typ is not found it falls back to Movie, which might be wrong.

It should check for all valid IMDB title types because if not found it falls back to Movie always.
@CicerBro
Copy link
Copy Markdown
Contributor Author

Hello @tboothman @jreklund. Would you be so kind as to merge this and tag a new version?

@duck7000
Copy link
Copy Markdown
Contributor

I wouldn't bother to try to revive this version as it seems abandoned.

If you want a working version with GraphQL check out my repo here
https://github.com/duck7000/imdbGraphQLPHP

And for the record this isn't a cry/try to lurk people to my github, i'm just trying to help to get a working version.

@CicerBro
Copy link
Copy Markdown
Contributor Author

Nice. But needs to be added to Packagist.

@duck7000
Copy link
Copy Markdown
Contributor

duck7000 commented Oct 21, 2024

Nope i'm not going to do that. My project is very simple so i don't think it needs all those bells and whistles.
Nobody so far has asked for this.

It ain't hard to download the source and extract it in your project is it?
And it has only 1 dependency that already are in most php installs

@duck7000
Copy link
Copy Markdown
Contributor

duck7000 commented Oct 21, 2024

nice thumbs down but that doesn't really help?
Why you insist on using packagist while this is a very simple project?
What is the benefit of using packagist?

I might consider to add it to packagist but i'm not convinced jet

fyi i have added my project to packagist

@CicerBro
Copy link
Copy Markdown
Contributor Author

Hello @tboothman @jreklund. Would you be so kind as to merge this and tag a new version?

@duck7000
Copy link
Copy Markdown
Contributor

If i may to point you on errors in your proposal..

Line 40,56 and 100 in your proposal is wrong, there shouldn't be a trailing comma for the last parameter

@CicerBro
Copy link
Copy Markdown
Contributor Author

Thats just formatting preferences. It works either way.

@duck7000
Copy link
Copy Markdown
Contributor

Thats just formatting preferences. It works either way.

I agree that this works either way but is not best practice and may be removed in future php versions

@jreklund
Copy link
Copy Markdown
Collaborator

jreklund commented Mar 1, 2025

It depends on the targeted PHP version. Support for trailing comma in functions and methods was added in PHP 7.3. As this library is supposed to support legacy PHP versions (PHP 5.6) this will crash on < PHP 7.3. I use trailing commas daily on multiline functions/methods, but this library can't use them.

https://php.watch/versions/7.3/func-call-trailing-comma

Officially only PHP 8.1 and newer is supported by the PHP Foundation but some LTS versions of Linux provide support for older versions and some companies offer paid solutions for supporting end of life versions.

@CicerBro
Copy link
Copy Markdown
Contributor Author

CicerBro commented Mar 1, 2025

Done

@jreklund jreklund merged commit 31597bb into tboothman:master Mar 2, 2025
@CicerBro
Copy link
Copy Markdown
Contributor Author

CicerBro commented Mar 2, 2025

Cheers. Could you tag a new version perhaps too?

@jreklund
Copy link
Copy Markdown
Collaborator

jreklund commented Mar 2, 2025

New version out. Took the time to updated the dev packages as well so they don't give off any warnings.

Plenty of things broken (as usual) and some are fixed in PR's, but will have to leave that for another day.

..........F..............F.FFEEFFFEF.FFFEFF.FFFF.FFFFFFF...FFF.  63 / 247 ( 25%)
..................................F...........F.F.........F..FF 126 / 247 ( 51%)
FF........FF...............FF..F....FFFF.....F................. 189 / 247 ( 76%)
.F..........FFFFFF.FFEEEF.F..F......EE.F.EEEFEEEEE.....F..      247 / 247 (100%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants