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

Channels: Add sort options to streams #4224

Merged
merged 8 commits into from
Jul 10, 2024

Conversation

src-tinkerer
Copy link
Contributor

@src-tinkerer src-tinkerer commented Nov 1, 2023

Uses the same method that is used for videos. We create continuation based on the sort option. One note though, produce_channel_livestreams_continuation and produce_channel_videos_continuation are almost identical, so is it better to simply make a singular function that produces the continuation based on the content?

@src-tinkerer src-tinkerer requested a review from a team as a code owner November 1, 2023 18:33
@src-tinkerer src-tinkerer requested review from SamantazFox and removed request for a team November 1, 2023 18:33
@SamantazFox
Copy link
Member

Yes, merging the two in a single function is strongly preferred.
Also, please remove produce_channel_livestream_url. It has no use (the function you copied, produce_channel_videos_url was only used to bypass captchas back then and will be discarded one day).

@src-tinkerer
Copy link
Contributor Author

I made the requested changes but couldn't test it yet because YouTube is blocked in my country, shouldn't be merged until then.

Copy link
Member

@SamantazFox SamantazFox left a comment

Choose a reason for hiding this comment

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

Code looks good! I'm only asking for a few style/syntax changes ^^

src/invidious/channels/videos.cr Outdated Show resolved Hide resolved
src/invidious/channels/videos.cr Outdated Show resolved Hide resolved
src/invidious/routes/channels.cr Outdated Show resolved Hide resolved
@src-tinkerer
Copy link
Contributor Author

Done, sorry for delay. Been busy with midterms.

@SamantazFox SamantazFox added the need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something label Nov 30, 2023
@SamantazFox
Copy link
Member

@src-tinkerer yeah, sure, no problem ^^ With my 60+ pending PRs to review, that'd be quite ironic for me to ask you to reply in a timely manner x)

@github-actions github-actions bot added the stale label Feb 29, 2024
@src-tinkerer
Copy link
Contributor Author

Will test this week

@github-actions github-actions bot removed the stale label Mar 17, 2024
@src-tinkerer
Copy link
Contributor Author

Okay done testing and live stream sort works.

@iv-org iv-org deleted a comment from github-actions bot Apr 27, 2024
@src-tinkerer

This comment has been minimized.

@SamantazFox SamantazFox added in-testing This feature has been deployed and is being tested and removed need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something labels May 13, 2024
@SamantazFox SamantazFox added ready and removed in-testing This feature has been deployed and is being tested labels Jun 16, 2024
@SamantazFox SamantazFox changed the title Add sort options to streams Channels: Add sort options to streams Jul 1, 2024
@SamantazFox SamantazFox merged commit bad9209 into iv-org:master Jul 10, 2024
@SamantazFox
Copy link
Member

Thanks for your contribution :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants