-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
refactor(ratelimit): Return Duration from Ratelimit functions #6283
base: main
Are you sure you want to change the base?
Conversation
Let's a bit unbend the `Ratelimit` interface: - It's a rather low-level Rust tool, so it should only use `Duration` and `SystemTime`. - Rename `update_interval()` to `min_send_interval()`, the word "update" only occurs in `last_update`, but it's a private field.
/// Should be exposed to `window.sendUpdateInterval` in JS land. | ||
pub send_update_interval: usize, | ||
pub send_update_interval: Duration, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope i don't break things here? This is Serialize
, but it seems that the true public API is the JSON-RPC one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best way to find out is adding a JSON-RPC test, I am also not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's already test_webxdc()
, but it tests WebxdcMessageInfo
defined in deltachat-jsonrpc/src/api/types/webxdc.rs
, not this struct.
Still, this WebxdcInfo
is public, so maybe there are some other implementations or core bindings using it, that's why i asked.
EDIT: Webxdc manifest files doesn't contain send_update_interval
, so this doesn't break parsing them at least.
@@ -72,7 +75,11 @@ impl WebxdcMessageInfo { | |||
source_code_url: maybe_empty_string_to_option(source_code_url), | |||
internet_access, | |||
self_addr, | |||
send_update_interval, | |||
send_update_interval: send_update_interval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the future i suggest to add the _ms
suffix, otherwise one can think it's in seconds. Moreover we have other intervals in API which are indeed in seconds
No description provided.