search

pimeys / rust-web-push

A Web Push library for Rust
Apache License 2.0
113 stars 21 forks source link

Allow specifying the `Urgency` of each push message #39

Closed hpeebles closed 1 year ago

hpeebles commented 1 year ago

This PR makes it easy to specify the message urgency as defined here - https://web.dev/push-notifications-web-push-protocol/#urgency

niklasf commented 1 year ago

Nice. Maybe also derive Deserialize and Serialize with #[serde(rename_all = "kebab-case")], for convenience?

hpeebles commented 1 year ago

Nice. Maybe also derive Deserialize and Serialize with #[serde(rename_all = "kebab-case")], for convenience?

Good plan, done!

niklasf commented 1 year ago

Ah, sorry, noticed one more thing while playing with it: Clone, Copy (and I guess even Eq, Ord, Hash, Default) would be useful, too.

hpeebles commented 1 year ago

Ah, sorry, noticed one more thing while playing with it: Clone, Copy (and I guess even Eq, Ord, Hash, Default) would be useful, too.

No worries! They are now added.

andyblarblar commented 1 year ago

Thanks for the clean PR! Just one last ask, would you mind adding the urgency header to this test: https://github.com/pimeys/rust-web-push/blob/84c8937e5ce3552658e4b1442fb6a09ff7847ce0/src/clients/request_builder.rs#L113

like how the TTL is being tested? I'd like to keep all these small headers tested to help with future sanity checks.

hpeebles commented 1 year ago

Thanks for the clean PR! Just one last ask, would you mind adding the urgency header to this test:

https://github.com/pimeys/rust-web-push/blob/84c8937e5ce3552658e4b1442fb6a09ff7847ce0/src/clients/request_builder.rs#L113

like how the TTL is being tested? I'd like to keep all these small headers tested to help with future sanity checks.

Done!

andyblarblar commented 1 year ago

Everything looks good to me. Thanks again for your contribution!

On another note, I've somehow missed the topic header up till now. It's probobly something we should consider adding in the future.