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

Remove existence checks and fallbacks for files that always exist #4544

Merged

Conversation

absidue
Copy link
Member

@absidue absidue commented Jan 14, 2024

Remove existence checks and fallbacks for files that always exist

Pull Request Type

  • Other - Performance Improvement

Description

Currently we check if the invidious fallback instances file, the external player data file and the geolocation files exist on the filesystem before loading them, however as they are in the source code repository and bundled with FreeTube, they will always exist unless someone goes out of the way to remove them from the FreeTube bundle or accidentally deletes them during development. So I think it's fine to remove the checks and fallbacks, because if they are actually missing, you'll actually get an error message, so that you know about the problem, instead of the silent fallback that we currently have.

For the geolocation files we do actually need to check if one exists in the display language, so that we can fallback to the en-US one if it doesn't, to keep that working I decided to copy the behaviour of the web build, which embeds a list of known files at build time, allowing use to just check the array instead of doing the runtime file system check every time.

Testing

  1. Change the display language and check that the list of regions for trending still gets updated
  2. Check that the external players list still gets populated in the external player settings.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.19.1

@absidue absidue marked this pull request as ready for review January 14, 2024 18:00
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 14, 2024 18:01
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 14, 2024
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Checked (1) but I can't check (2)

Copy link
Member

Choose a reason for hiding this comment

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

In the same boat as Pika

@absidue
Copy link
Member Author

absidue commented Feb 4, 2024

Actually all you need to do is check that the external players list is still populated in the settings, you don't actually meed to try it (if reading the file were to fail the list would be empty anyway).

Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

yup didn't read code

@FreeTubeBot FreeTubeBot merged commit e0942ea into FreeTubeApp:development Feb 4, 2024
11 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Feb 4, 2024
@absidue absidue deleted the remove-known-file-fallbacks branch February 4, 2024 20:48
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.

5 participants