-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Fix trying to delete object not in list #6127
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java
Outdated
Show resolved
Hide resolved
@Redirion See it shows "Failing after 49s - build-and-test-jvm" |
ah found the reason. Please modify the checkstyle-suppressions.xml it contains: the line numbers need to be adjusted accordingly |
What if in future line number changes again ? It will be back again :( |
Checkstyle is stupid in this regard. In my opinion it should not enforce "final" modifier for primitives and String, as String is immutable. |
Any updates on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it still feels like fixing the symptom but not the cause. But it is better than crashing and its well commented.
What is it?
Description of the changes in your PR
-When the removeItem() is called it might happen somehow that object is not in list , in this case indexOf() will return -1 and in this case we should not try to delete object by it's index (i.e is -1)
-Coming to "somehow" this may happen when
1)the removeItem() is called on duplicate of item in list , in this case we need to implement a way to remove item by it's duplicate as duplicate is not in list
2)the object is already deleted and the UI is not updated (handled this in this PR)
Fixes the following issue(s)
Relies on the following changes
APK testing
On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.
Due diligence