-
Notifications
You must be signed in to change notification settings - Fork 2.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
Update to FSRS-rs v0.6.1 #3106
Update to FSRS-rs v0.6.1 #3106
Conversation
Looks good |
These tooltips should also update. @Expertium, @user1823, any suggestion? Lines 393 to 396 in 3cd8c57
Lines 413 to 418 in 3cd8c57
|
I suggest just replacing 1000 with 8. Also, the same needs to be done with the "insufficient review history" error message. |
The user still could click |
Maybe one option would be to stop showing an error, and just fall back on the default parameters instead? |
That would be quite simple, which is nice, but it could cause confusion. If the user clicks "Optimize" and parameters don't change, he will think that the optimizer is broken. |
If we don't change anything, we can keep showing the message that the parameters are currently optimal. |
Is there any specific reason why the parameters need to be shown to users? That section also looks quite cluttered in the Android app so one less thing there if it's not shown. |
Some power users like to be able to see and/or modify them. |
@dae, do you plan to release 24.04.1 after you merge this pull request? |
.1 will be a quick update with only some bugfixes, so I wasn't planning to include this. There may be a .2 if we have a longer testing period to try it out. |
Thanks @L-M-Sherlock. This will go into the main branch, but probably not into the 24.04.1 branch. |
related update:
close #3094
@user1823, @Expertium, @RlzHi, please review this PR.