Mutate edns & remove edns options #1363
Conversation
leshow
force-pushed
the
mutate_edns
branch
2 times, most recently
from
1264b06
to
11569bb
Compare
crates/proto/src/op/edns.rs
Outdated
|
|
||
| /// Remove the option specified by the `EdnsCode` | ||
| pub fn remove_option(&mut self, option: EdnsCode) { | ||
| self.options.remove(option); | ||
| } |
There was a problem hiding this comment.
Do you actually have a use case for this? It seems redundant with options_mut(), and I think I'd personally prefer only to expose options_mut() instead of having both.
There was a problem hiding this comment.
Yeah, I wanted to get your take on this actually. It is redundant, but so is set_option once we have options_mut. The only reason I added remove_ was so there was some symmetry in the API.
It doesn't matter to me either way though, as long as there's some way to accomplish a removal.
edit: If we do with options_mut would you want to leave a #[deprecated] on set_options? Or just leave the method there?
There was a problem hiding this comment.
Yeah, adding #[deprecated] on set_option() sounds like a plan to me.
|
(Also please rebase on current main.) |
leshow
force-pushed
the
mutate_edns
branch
2 times, most recently
from
8811329
to
e4ad3dd
Compare
|
Blah sorry, I kept pushing with a few typos. There it should all be fine now. |
Codecov Report
@@ Coverage Diff @@
## main #1363 +/- ##
==========================================
+ Coverage 86.29% 86.30% +0.01%
==========================================
Files 132 132
Lines 13775 13788 +13
==========================================
+ Hits 11886 11899 +13
Misses 1889 1889 |
|
Looks like some |
|
Thanks! |
closes #1362