Add more tests, remove old comments #100
Conversation
Codecov Report
@@ Coverage Diff @@
## main #100 +/- ##
==========================================
+ Coverage 69.88% 72.00% +2.11%
==========================================
Files 67 67
Lines 5217 5354 +137
==========================================
+ Hits 3646 3855 +209
+ Misses 1571 1499 -72
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| }*/ | ||
| /*impl HasRegistryEventString for PageChangedEvent { | ||
| const REGISTRY_EVENT_STRING: &'static str = "Document:PageChanged"; | ||
| }*/ | ||
| impl HasRegistryEventString for DocumentEvents { |
There was a problem hiding this comment.
Good to see the commented blocks go!
| @@ -52,26 +73,26 @@ impl<'de> Deserialize<'de> for Interface { | |||
| E: de::Error, | |||
| { | |||
| match value { | |||
There was a problem hiding this comment.
Why replace the literals "org.a11y.atspi.Event.Accessible", with constants.
What is the advantage over implementing Display on the Interface enum and use that variant where we need it?
I know it was not in this commit but would you know why a manual implementation of Serialize and Deserialize? Trying to think of a reason but if we had TryFrom<&str> for Interface and the From<Interface> for &str would that allow for deriving?
Can we try? If possible I would choose derived serde impls over manual ones.
There was a problem hiding this comment.
Trying to think of a reason but if we had TryFrom<&str> for Interface and the From for &str would that allow for deriving?
It would still require a manual implementation of serde's Serialize and Deserialize traits, it's just that we'd be able to take advantage of existing conversions.
I know it was not in this commit but would you know why a manual implementation of Serialize and Deserialize?
I suspect that it was because there's no easy way to set the value of an enum variant to an arbitrary string with serde?
There was a problem hiding this comment.
Why replace the literals "org.a11y.atspi.Event.Accessible", with constants.
I have had issues before with a mis-matched name in one place not matching another. Using constants avoids any ambiguity. They either work everywhere, or work nowhere.
No description provided.