Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Convert Sched2Test to Kotlin #11802

Merged
merged 2 commits into from
Jul 10, 2022
Merged

Convert Sched2Test to Kotlin #11802

merged 2 commits into from
Jul 10, 2022

Conversation

dae
Copy link
Contributor

@dae dae commented Jul 8, 2022

Useful for the backend work I'm working on

mikehardy
mikehardy approved these changes Jul 8, 2022
View reviewed changes
Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

seems fine w/two cleanup annotation questions - one definite, one I asked David on. Happy to see this go in quickly to keep from blocking progress, cleanups are minor points

AnkiDroid/src/test/java/com/ichi2/libanki/sched/SchedV2Test.kt
assertThat("Repeating single step - 20 minutes * 1.5", two, is(1800L)); // 30 mins
assertThat("Good should take the reduced interval (25 * 0.7)", three, is(1468800L)); // 17 days
assertThat("Easy should have a bonus day over good", four, is(1555200L)); // 18 days
MatcherAssert.assertThat(after.type, Matchers.`is`(Consts.CARD_TYPE_RELEARNING))
Copy link
Member

Choose a reason for hiding this comment

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

KotlinCleanup is

AnkiDroid/src/test/java/com/ichi2/libanki/sched/SchedV2Test.kt
note.setItem("Back", "two")
col.addNote(note)
col.reset()
Assert.assertEquals(1, col.sched.newCount().toLong())
Copy link
Member

Choose a reason for hiding this comment

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

@david-allison I seem to recall other KotlinCleanups around toLong? is there one needed for this class?

@mikehardy mikehardy added Needs Author Reply Waiting for a reply from the original author Review High Priority Request for high priority review Needs Second Approval Has one approval, one more approval to merge Backend Related May have something to do with Rust, related to Anki-Android-Backend labels Jul 8, 2022
@dae dae mentioned this pull request Jul 9, 2022
Merged
lukstbit
lukstbit approved these changes Jul 9, 2022
View reviewed changes
Copy link
Member

@lukstbit lukstbit left a comment

Choose a reason for hiding this comment

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

Looks good, with one issue. Add a kotlin cleanup on the SchedV2Test class:

@KotlinCleanup("fix ide lint issue and improve kotlin code where possible")

dae added 2 commits July 10, 2022 12:37
@dae
refactor: Rename SchedV2Test.java to .kt
a3727e9 
com.ichi2.libanki.sched.SchedV2Test
@dae
refactor: Convert SchedV2Test to Kotlin
cd732df 
com.ichi2.libanki.sched.SchedV2Test
@dae
Copy link
Contributor Author

dae commented Jul 10, 2022

Done

@lukstbit lukstbit added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge Needs Author Reply Waiting for a reply from the original author labels Jul 10, 2022
@mikehardy mikehardy merged commit 02d7148 into ankidroid:main Jul 10, 2022
@github-actions github-actions bot removed Review High Priority Request for high priority review Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) labels Jul 10, 2022
@github-actions github-actions bot added this to the 2.16 release milestone Jul 10, 2022
@dae dae mentioned this pull request Jul 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukstbit lukstbit lukstbit approved these changes

@mikehardy mikehardy mikehardy approved these changes

@david-allison david-allison Awaiting requested review from david-allison

Assignees
No one assigned
Labels
Backend Related May have something to do with Rust, related to Anki-Android-Backend
Projects
None yet
Milestone
2.16 release
Development

Successfully merging this pull request may close these issues.
3 participants
@dae @mikehardy @lukstbit