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

[Arte7Bridge] Support all languages #2543

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

tpikonen
Copy link
Contributor

This PR adds support for all languages in arte.tv. I've used this for a while, and it works ok, at least for some value of 'works'.

Based on code by mitsukarenai from
https://gist.github.com/mitsukarenai/93c12e1470e2b731d64a31dffbe46075

This has never been tested with the web interface, I have generated local rss-files with commands like this:

/usr/bin/php index.php action=display bridge=Arte7 format=Mrss lang=en cat='' > arte_all.rss
/usr/bin/php index.php action=display bridge=Arte7 format=Mrss lang=en col=RC-020782 > making_history.rss

I basically don't know PHP at all, so I'm not going to maintain this code.

Also increases the limit to 100.

Fixes #1906.

github-actions bot added a commit that referenced this pull request Mar 26, 2022
github-actions bot added a commit that referenced this pull request Mar 26, 2022
github-actions bot added a commit that referenced this pull request Mar 26, 2022
github-actions bot added a commit that referenced this pull request Mar 26, 2022
github-actions bot added a commit that referenced this pull request Mar 26, 2022
github-actions bot added a commit that referenced this pull request Mar 26, 2022
@github-actions
Copy link

github-actions bot commented Mar 26, 2022

Pull request artifacts

file last change
Arte7-current-context1 2022-03-28, 19:59:37
Arte7-current-context2 2022-03-28, 19:59:37
Arte7-current-context3 2022-03-28, 19:59:37
Arte7-current-context4 2022-03-28, 19:59:37
Arte7-pr-context1 2022-03-28, 19:59:37
Arte7-pr-context2 2022-03-28, 19:59:37

Copy link
Contributor

@Bockiii Bockiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the limit, this looks fine.

This will break backwards compatibility, but I am a fan of making the bridge easier to maintain (no specific contexts for each language) and also adding basically 200% more languages.

I'll let @em92 decide because of seniority. But I think if we do this big cleanup, a few bridges backwards compat will be broken. And this bridge pr definitely comes with benefits.

break;
}

$url = 'https://api.arte.tv/api/opa/v3/videos?sort=-lastModified&limit=10&language='
$url = 'https://api.arte.tv/api/opa/v3/videos?sort=-lastModified&limit=100&language='
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$url = 'https://api.arte.tv/api/opa/v3/videos?sort=-lastModified&limit=100&language='
$url = 'https://api.arte.tv/api/opa/v3/videos?sort=-lastModified&limit=15&language='

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely not 100. Even 15 cover more than a month in all tested contexts (see bot commet). There is no reason to pull 100 items on every refresh.

@tpikonen
Copy link
Contributor Author

Thanks, I changed the limit to 15.

@Bockiii
Copy link
Contributor

Bockiii commented Mar 31, 2022

We've broken multiple backwards compatibilities for the cleanup action, so I'm merging this.

@Bockiii Bockiii merged commit aa0aa72 into RSS-Bridge:master Mar 31, 2022
Kwbmm pushed a commit to Kwbmm/rss-bridge that referenced this pull request Jun 17, 2022
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.

Arte English
2 participants