fix: reminder question time elapsed#17774
fix: reminder question time elapsed#17774gayatriii0803 wants to merge 1 commit intoankidroid:mainfrom
Conversation
2edd15d to
447cd4d
Compare
|
I don't understand why this fails. Is this related to my code? |
|
@gayatriii0803 failure most likely unrelated:
Github blew up https://www.githubstatus.com/incidents/l76qh74ryp16
|
|
Closing and reopening forces a CI re-run. |
447cd4d to
6bcda80
Compare
6bcda80 to
f8c5f92
Compare
Can anyone help me here? |
|
@david-allison - the test using |
|
JVM - the backend shouldn't need an Android dependency |
|
Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically |
f8c5f92 to
9c33827
Compare
| @Test | ||
| fun fromPreferenceValue() { | ||
| @RunWith(AndroidJUnit4::class) | ||
| class AutomaticAnswerActionTest : RobolectricTest() { |
There was a problem hiding this comment.
why do you need robolectric here? You can use a generic message instead of TR. below
david-allison
left a comment
There was a problem hiding this comment.
I don't understand the context of the bug from this PR
Could the commit message be updated to give a very brief explanation of the issue and the fix
mikehardy
left a comment
There was a problem hiding this comment.
seems like it's good to go but agreed, the PR description doesn't really say why it wasn't working before and why the PR fixes it, and the comment doesn't either.
Both should have some sort of <feature> wasn't working because of <reason>, this PR solves it by <approach> so that future people know what is going on
I can't really merge it without feeling like I know what is going on
BrayanDSO
left a comment
There was a problem hiding this comment.
I tested that on the new reviewer instead of the old one 🙈, and I already added that reminder since the implementation of Auto Advance there. Shame on me.
Your PR mistakenly uses the same action of the answer
87db28a to
9fc8155
Compare
9fc8155 to
c58c164
Compare
c58c164 to
0ac44b0
Compare
| fun execute(reviewer: Reviewer) { | ||
| val action = this.toCommand() | ||
| if (action != null) { | ||
| Timber.i("Executing %s", action) | ||
| reviewer.executeCommand(action) | ||
| } else { | ||
| reviewer.showSnackbar(TR.studyingQuestionTimeElapsed()) | ||
| } | ||
| } | ||
|
|
||
| /** Convert to a [ViewerCommand] */ | ||
| private fun toCommand(): ViewerCommand? = | ||
| when (this) { | ||
| SHOW_ANSWER -> ViewerCommand.SHOW_ANSWER | ||
| SHOW_REMINDER -> null | ||
| } | ||
|
|
There was a problem hiding this comment.
this couples the class to the old reviewer, which is bad by itself as code design, and the old reviewer will be removed eventually. Please do these changes on Reviewer or extensions methods there, but not here.
0ac44b0 to
fb9d055
Compare
|
Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically |
|
Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically |


Purpose / Description
Auto Advance "Question time elapsed" reminder does not work
Fixes
Approach
show snackbar on
automaticShowAnswer()How Has This Been Tested?
Physical device (OPPO F21 Pro 5G).
Checklist
Please, go through these checks before submitting the PR.