feat(Preferences/SponsorBlock): drop VISIBLE option
#7434
Conversation
Introduces a new function, that checks for preferences, that need to be migrated, and migrates them if necessary.
Drops the `VISIBLE` option for SponsorBlock, as it provides no additional value and leads to bad UX, as a user is able to see the segment, but able to skip it. Existing users are migrated to the `MANUAL` option. Ref: libre-tube@f5c4015 Ref: libre-tube@9ceb258
| } | ||
|
|
||
| // mark as successfully migrated | ||
| putInt(PreferenceKeys.PREFERENCE_VERSION, BuildConfig.VERSION_CODE) |
There was a problem hiding this comment.
Not sure if it's better to use the version code here, or a separate constant, to avoid having the migrations run on every startup for builds where the version is not increased on every update, e.g. Debug/Nightly builds?
There was a problem hiding this comment.
I think it's better to manually increment the number, because not every version will have a preference migration, so it'd run the whole function body unnecessarily with each update even if there's no migration necessary.
Depending on how much we want to over-engineer it: In theory we could build a simple PreferenceMigration class similar to the room migrations and maintain a list of migrations numbered from 1 upwards, see https://github.com/libre-tube/LibreTube/blob/master/app/src/main/java/com/github/libretube/db/AppDatabase.kt. We could then have a PreferenceMigration(1, 2) { override fun onMigrate() {} }, that runs the migration.
There was a problem hiding this comment.
Implemented in c746647.
This system allows to run migrations even without changing version codes, allowing for mid-version preferences changes to are applied to debug/nightly builds.
|
Thinking about it, maybe it also makes sense to combine |
We already have the icon to "temporarily" allow SponsorBlock segments, so the "automatic once" behavior could be achieved by temporarily allowing segments, yes. It would be confusing for the average people if the segment doesn't skip again and again by default, so I however think that it'd make sense to make some kind of community poll or open an issue about it to see if there are many users of this feature, because otherwise people will start complaining about yet another removed feature again... |
2 participants
Drops the
VISIBLEoption for SponsorBlock, as it provides no additional value and leads to bad UX, as a user is able to see the segment, but able to skip it.Existing users are migrated to the
MANUALoption. To achieve this, new supporting code is introduced, that allows us to automatically migrate preferences.Ref: #7384 (comment)
Ref: f5c4015
Ref: 9ceb258