Skip to content

Fix #20763: Match Scheduler.answerCard with upstream Anki#21162

Closed
vivek41-glitch wants to merge 6 commits into
ankidroid:mainfrom
vivek41-glitch:fix/upstream-scheduler-match
Closed

Fix #20763: Match Scheduler.answerCard with upstream Anki#21162
vivek41-glitch wants to merge 6 commits into
ankidroid:mainfrom
vivek41-glitch:fix/upstream-scheduler-match

Conversation

@vivek41-glitch
Copy link
Copy Markdown
Contributor

Issue

#20763 - NoSuchElementException when cardsList is empty

Root Cause

Scheduler.kt deviated from upstream by using queuedCards.cardsList.first()

Fix

Replace with col.backend.getSchedulingStates(card.id) to match Python implementation

Changes

  • Removed invalid queuedCards.first() call
  • Now matches upstream behavior exactly
  • Prevents NoSuchElementException

Testing

Code matches upstream Anki at commit 5e46fc4494b428387f1d3f5c19d0ed19a089705e

@david-allison
Copy link
Copy Markdown
Member

Cheers!

Please drop the irrelevant commits and the merge commit. Prefer to have multiple commits which we can rebase onto main

https://github.com/ankidroid/Anki-Android/blob/main/CONTRIBUTING.md#before-submitting-a-pull-request-pr

@vivek41-glitch
Copy link
Copy Markdown
Contributor Author

hi @david-allison , in my terminal things get messy ,it become overcomplicated from my side . would do it from urself squashing when merging ? i will appreciate
thanks .

@david-allison
Copy link
Copy Markdown
Member

Being able to lint your code, either via our pre-commit hooks, or./gradlew ktLintFormat is a requirement to contribute.

@criticalAY
Copy link
Copy Markdown
Contributor

You changed the PR template. Please stick to the original template and fill details there

@vivek41-glitch
Copy link
Copy Markdown
Contributor Author

You changed the PR template. Please stick to the original template and fill details there

@criticalAY Thank you for the feedback. I apologize for changing the template. I have created a new PR (#21163) with the correct code. Please review that one instead. I will close this PR.

@david-allison
Copy link
Copy Markdown
Member

Please force push rather than creating new pull requests - it's more work for us, and we lose the conversation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants