Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

implement serde for Key Binding #9198

Merged
merged 3 commits into from
Jul 10, 2021

Conversation

david-allison
Copy link
Member

@david-allison david-allison commented Jul 4, 2021

Pull Request template

Purpose / Description

Key Bindings need to be serialized and deserialized from preferences

This brings in the code for #8078 to handle this aspect of #6052

How Has This Been Tested?

Should be non-functional, unit tests lifted from the PR

Checklist

  • You have not changed whitespace unnecessarily (it makes diffs hard to read)
  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • Your code follows the style of the project (e.g. never omit braces in if statements)
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@david-allison david-allison added the Blocked by dependency Currently blocked by some other dependent / related change label Jul 4, 2021
@david-allison

This comment has been minimized.

@david-allison david-allison marked this pull request as ready for review July 4, 2021 16:49
@david-allison david-allison added Needs Review and removed Blocked by dependency Currently blocked by some other dependent / related change labels Jul 4, 2021
@david-allison david-allison changed the title DRAFT: implement serde for Key Binding implement serde for Key Binding Jul 4, 2021
@david-allison david-allison added Needs Author Reply Waiting for a reply from the original author and removed Needs Review labels Jul 9, 2021
@david-allison david-allison self-assigned this Jul 9, 2021
@david-allison david-allison added Needs Review and removed Needs Author Reply Waiting for a reply from the original author labels Jul 9, 2021
For serialisation to preferences

From PR 8078 for 6052
@mikehardy
Copy link
Member

Unit test failure looks legit ?

        Caused by:
        java.lang.AssertionError: expected:<com.ichi2.anki.reviewer.PeripheralKeymap$MappableBinding@8c1b> but was:<com.ichi2.anki.reviewer.PeripheralKeymap$MappableBinding@745f>
            at org.junit.Assert.fail(Assert.java:89)
            at org.junit.Assert.failNotEquals(Assert.java:835)
            at org.junit.Assert.assertEquals(Assert.java:120)
            at org.junit.Assert.assertEquals(Assert.java:146)
            at com.ichi2.anki.reviewer.BindingAndroidTest.assertBindingEquals(BindingAndroidTest.kt:56)
            at com.ichi2.anki.reviewer.BindingAndroidTest.testFromString(BindingAndroidTest.kt:42)

For deserialization to preferences

From PR 8078 for 6052
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.

LGTM! Also, because of this I learned of Rust "traits" via lookup of "what is serde" and seeing serde.rs 🤓

@mikehardy mikehardy merged commit b1eeed1 into ankidroid:master Jul 10, 2021
@mikehardy mikehardy added this to the 2.16 release milestone Jul 10, 2021
@david-allison david-allison deleted the binding-string branch July 11, 2021 00:02
@david-allison
Copy link
Member Author

LGTM! Also, because of this I learned of Rust "traits" via lookup of "what is serde" and seeing serde.rs 🤓

modem = modulator-demodulator
serde = serialiser-deserialiser

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants