search

Unleash / unleash-client-rust

Unleash client SDK for Rust language projects
Apache License 2.0
23 stars 17 forks source link

feat: implement semver strategy constraints #65

Open Meemaw opened 1 year ago

Meemaw commented 1 year ago

About the changes

Implements semver based strategy constraints.

Closes https://github.com/Unleash/unleash-client-rust/issues/64 Closes https://github.com/Unleash/unleash-client-rust/issues/35

There a re a bunch of other constraints still not implemented, that should be trivial to add in a followup. For now, this PR has a bit more logic due to the semver constraint using the "value" field instead of "values" which requires serde to be aware of that.

Edit: I see there is a ~1 year old PR (https://github.com/Unleash/unleash-client-rust/pull/27) that implements constraints as well. Wondering what's up with that.

Important files

Discussion points

Meemaw commented 1 year ago

@sighphyre @thomasheartman is there any plan to merge/release this, or https://github.com/Unleash/unleash-client-rust/pull/27 in near future? This is currently blocking us from using unleash in a rust project. Can publish a new crate to unblock, but really don't want to do that for a feature that could benefit everyone.

thomasheartman commented 1 year ago

@Meemaw I think we'd love to get this out, yeah. I don't know exactly what happened to @sighphyre's PR from a year ago (#27), but there is probably a reason why it wasn't merged. Maybe either he or @rbtcollins could shed some light on that.

I don't know what's holding us back (it might just be development time), but I know this repo is still using a client spec from three years ago. I've got a feeling that we'd also want to update that when moving forwards, but I'm not sure it's necessary before merging this.

As you can maybe tell, I don't have as much context around this particular SDK as @sighphyre and @rbtcollins do, so I'll defer to them.

sighphyre commented 1 year ago

Hey @Meemaw, there's no concrete reason we haven't moved on that, it was just a case of other work becoming more important and remaining more important until right now. To date we haven't had anyone ask for these features in the Rust SDK and other SDK work was prioritised

Now that people are asking it can be reprioritized but I probably won't look at least until early next week

Meemaw commented 1 year ago

@sighphyre thanks for the context. I think it wouldn't be as bad if the SDK worked and just ignored those constraints and treat them as a noop, but the fact that it doesn't work at all with these constraints in the project makes it very hard to use.

I'll check in again next week to see if you can prioritise this.

Meemaw commented 1 year ago

Hey @sighphyre as promised circling back to check if there is a plan to prioritise this soon/this week? If not that's fine and we will work-around the issue for now.

sighphyre commented 1 year ago

Hey @Meemaw, still planning to prioritise this soon but it might not be this week. I have some relatively urgent things to work on in another SDK, as soon as that's done this will be the next thing.