feat: add support for namePrefix, tags and project filters
#54
Conversation
There was a problem hiding this comment.
@teqm Thanks so much for the contribution! It would be nice to get this feature into this SDK. I have some nitpicks here and I need a bit of time to review a bit more carefully but I'd like @rbtcollins to weigh in on the overall approach before I do, since he's much more familiar with this SDK than I am
|
...but also, Clippy seems to have a few warnings here |
src/api.rs
Outdated
| } | ||
| } | ||
|
|
||
| #[cfg(not(feature = "surf"))] |
There was a problem hiding this comment.
these parts should be written against the local http client trait, so that both surf and reqwest users gain them. That trait may need tweaking to accomodate your needs.
There was a problem hiding this comment.
I couldn't find a "nice" way of implementing it 🤔 I didn't want to polute the trait with the logic of features query, so instead I made it that the query is already present in the url when it's passed to the http layer. wdyt is it fine to do it that way?
It adds a convenient way to filter the features - yeah I agree with you that it doesn't bring a lot of value, but if I'm not mistaken the tag filtering isn't possible to implement client side as this information isn't returned from the API. When it comes to If you feel like this isn't valuable, feel free to reject the PR - I can handle it 😅 |
About the changes
Added missing support for feature filters, that is
namePrefix,tagsandproject. According to https://docs.getunleash.io/reference/sdks#server-side-sdk-compatibility-table it's already present in some of the SDKs, so I suppose we can add it here as well. It's nothing "big", but in my experience (with node.js SDK) can be useful.Important files
Discussion points