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.

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

Update Priority de/serialization #11

Merged
merged 3 commits into from
Jan 13, 2025
Merged

Update Priority de/serialization #11

merged 3 commits into from
Jan 13, 2025

Conversation

yukibtc
Copy link
Member

@yukibtc yukibtc commented Jan 12, 2025

De/serialize Priority from/to string if the form is human-readable, otherwise to u8.

@yukibtc
Copy link
Member Author

yukibtc commented Jan 12, 2025

@JoeJoeTV

@yukibtc
Copy link
Member Author

yukibtc commented Jan 12, 2025

Seems that doesn't work anymore, probably the JSON payload must contain the priority as number.

@JoeJoeTV
Copy link
Contributor

I also just checked, what is used for the JSON (de-)serializer:
The serde_json crate, which is used for this uses the default trait implementation for the is_human_readable function, which specifies that it is human readable, so If you're okay with it using a string for JSON, then this looks good to me.

@yukibtc
Copy link
Member Author

yukibtc commented Jan 12, 2025

The ntfy server return error 400 when sending the priority as string

@JoeJoeTV
Copy link
Contributor

The ntfy server return error 400 when sending the priority as string

Yeah, the message format seems to specify an integer: https://docs.ntfy.sh/subscribe/api/#json-message-format

@JoeJoeTV
Copy link
Contributor

JoeJoeTV commented Jan 12, 2025

Then maybe just allow strings in the deserializer, as I mentioned here and "force" u8 for serialization.

EDIT: I just realized that would still be a problem sometimes. Even if you won't keep the string in the (de-)serialization, having the as_str and from_str is still useful. I'll check how this works with serde attributes.

EDIT2: I also just remembered why this was a bigger problem. I have the Priorities in a hash map, so I can't directly use the attribute to specify a custom (de-)serialization function without replacing the one used for the hash map itself.

Support both deserialization from `u8` and `String`.

Signed-off-by: Yuki Kishimoto <[email protected]>
@yukibtc
Copy link
Member Author

yukibtc commented Jan 12, 2025

@JoeJoeTV, now the Priority support both deserialization from u8 and String

@JoeJoeTV
Copy link
Contributor

JoeJoeTV commented Jan 12, 2025

@JoeJoeTV, now the Priority support both deserialization from u8 and String

Alright, thanks, nice, I'll try this with my usecase.

@JoeJoeTV
Copy link
Contributor

@yukibtc This seems to work well! Thanks!

@yukibtc yukibtc merged commit 65c12df into master Jan 13, 2025
2 checks passed
@yukibtc yukibtc deleted the priority branch January 13, 2025 08:22
@yukibtc
Copy link
Member Author

yukibtc commented Jan 13, 2025

@JoeJoeTV I think we are ready for v0.7, right?

@JoeJoeTV
Copy link
Contributor

Yes, I would say so.

@yukibtc
Copy link
Member Author

yukibtc commented Jan 13, 2025

@JoeJoeTV, I've just released v0.7

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

Successfully merging this pull request may close these issues.

2 participants