search

keepsimple1 / mdns-sd

Rust library for mDNS based Service Discovery
Apache License 2.0
96 stars 37 forks source link

Use `serde` for de/ser Txt Records #166

Closed r4v3n6101 closed 8 months ago

r4v3n6101 commented 8 months ago

Hi there! Could we start to use serde for de/ser of TXT properties? To be honest, I'm not sure if it's RFC-complaint, may someone more experienced answer me.

keepsimple1 commented 8 months ago

Thanks for opening an issue!

Could you please share what are the problems you see in the current implementation of de/ser of TXT properties? (e.g. decode_txt. It refers to the RFC 6763 section 6.1 and onwards.

serde is a relatively big library on its own that will bring in many other dependencies, including proc-macro2, syn, etc. That will also increase the compile time visibly. We have to have strong reasons to use them.

r4v3n6101 commented 8 months ago

Personally, I think it was a good idea. But actually, seems like it's not. I don't count in 'serde' size, just thinking in terms of standards (and 'serde' is kinda standard for such formats)

keepsimple1 commented 8 months ago

Regarding your question of RFC-compliant, we have been pragmatic in the implementation. Hence we do not cover every part (e.g. SHOULD) of the RFC. But if you have a user case that needs improvement / changes of TXT property processing, please raise them and also feel free to open a PR.

Also, if some points marked as MUST in the RFC but not covered by the current implementation, please point it out and we shall fix them.

r4v3n6101 commented 8 months ago

Well, I've dropped an eye to the RFC. I was wrong about 'serde', it looks like overkill feature My apologize for this strange question, I'll close it