Skip to content

remove reference to Preferences of deleting category#1734

Merged
AntsyLich merged 6 commits intomihonapp:mainfrom
cuong-tran:remove-unrefered-category-pref
Feb 24, 2025
Merged

remove reference to Preferences of deleting category#1734
AntsyLich merged 6 commits intomihonapp:mainfrom
cuong-tran:remove-unrefered-category-pref

Conversation

@cuong-tran
Copy link
Copy Markdown
Contributor

When a category is deleted, preferences which are referencing to that category should also be clear to avoid null/incorrect settings

Copy link
Copy Markdown
Member

@AntsyLich AntsyLich left a comment

Choose a reason for hiding this comment

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

Inject in the class constructor also add a migration to clean existing preferences? And changelog.

@cuong-tran
Copy link
Copy Markdown
Contributor Author

add a migration to clean existing preferences?

A migration seems overkill. These preferences can be easily fixed once user notices and re-set them again.

@MajorTanya
Copy link
Copy Markdown
Member

add a migration to clean existing preferences?

A migration seems overkill. These preferences can be easily fixed once user notices and re-set them again.

Never rely on user action to fix incorrect state.

@BrutuZ
Copy link
Copy Markdown
Contributor

BrutuZ commented Feb 18, 2025

once user notices

Overestimating much? :risitas:

@cuong-tran
Copy link
Copy Markdown
Contributor Author

because this referred settings issue been there since long but users didn't really have problem with it that much, meaning this issue is not a critical one.
Migration, in other hand, requires bumping versionCode and hence prevent user from going back version.

@MajorTanya
Copy link
Copy Markdown
Member

because this referred settings issue been there since long but users didn't really have problem with it that much, meaning this issue is not a critical one.
Migration, in other hand, requires bumping versionCode and hence prevent user from going back version.

How does it prevent uninstalling and installing an old version? It doesn't. And going back to older versions isn't a supported thing for Mihon either. We make it a point in support requests and issues here to make sure people use the latest version before raising complaints.

@cuong-tran
Copy link
Copy Markdown
Contributor Author

because this referred settings issue been there since long but users didn't really have problem with it that much, meaning this issue is not a critical one.
Migration, in other hand, requires bumping versionCode and hence prevent user from going back version.

How does it prevent uninstalling and installing an old version? It doesn't. And going back to older versions isn't a supported thing for Mihon either. We make it a point in support requests and issues here to make sure people use the latest version before raising complaints.

it prevents install over (to keep data/cache). Anw, it's opinion because I only said overkill, given the minor affect of this issue comparing to bumping versionCode.

@AntsyLich
Copy link
Copy Markdown
Member

You do realize I'm going bump the version on new release anyway?

@cuong-tran
Copy link
Copy Markdown
Contributor Author

You do realize I'm going bump the version on new release anyway?

as a matter of fact, no I don't, because there are so little updates so I honestly couldn't notice. anw I'm doing migration now.

@AntsyLich
Copy link
Copy Markdown
Member

You do realize I'm going bump the version on new release anyway?

as a matter of fact, no I don't, because there are so little updates so I honestly couldn't notice. anw I'm doing migration now.

Oi that's a foul :angy:

Copy link
Copy Markdown
Member

@AntsyLich AntsyLich left a comment

Choose a reason for hiding this comment

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

Also bump app version to 10

Comment thread CHANGELOG.md Outdated
Comment thread domain/src/main/java/tachiyomi/domain/category/interactor/DeleteCategory.kt Outdated
Comment thread domain/src/main/java/tachiyomi/domain/category/interactor/DeleteCategory.kt Outdated
@AntsyLich AntsyLich merged commit eeb6830 into mihonapp:main Feb 24, 2025
@cuong-tran cuong-tran deleted the remove-unrefered-category-pref branch February 25, 2025 09:59
cuong-tran added a commit to komikku-app/komikku that referenced this pull request Feb 25, 2025
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants