search

openrewrite / rewrite-feature-flags

OpenRewrite recipes for LaunchDarkly.
Apache License 2.0
2 stars 2 forks source link

Add support for `migrationVariation` #17

Closed shanman190 closed 9 months ago

shanman190 commented 9 months ago

What problem are you trying to solve?

Add support for the migrationVariation to the FindFeatureFlag search recipe. https://launchdarkly.github.io/java-server-sdk/com/launchdarkly/sdk/server/LDClient.html#migrationVariation(java.lang.String,com.launchdarkly.sdk.LDContext,com.launchdarkly.sdk.server.MigrationStage)

Describe the solution you'd like

Search recipe appropriately discovered the new variation type.

Additional context

This is a new variation method as of 7.x. more information on it can be found here: https://docs.launchdarkly.com/guides/flags/migrations

Are you interested in contributing this feature to OpenRewrite?

Always. :)

timtebeek commented 9 months ago

I'm guessing we already pick this up when we use flagType = null due to com.launchdarkly.sdk.server.LDClient *Variation(..), but you want to add this as a valid option? That would make this a very quick one I suppose, or did I miss anything? https://github.com/openrewrite/rewrite-launchdarkly/blob/d3136c86edbab65c967583a7264e55d072e0764e/src/main/java/org/openrewrite/launchdarkly/search/FindFeatureFlag.java#L43-L49

shanman190 commented 9 months ago

Absolutely correct! Just allowing the user to select the new variation type is all that is necessary here. When not selected, it just kinda works already like you mentioned. This is really just enabling the user to scope down, if they have that desire.

shanman190 commented 9 months ago

A question that comes to mind as another alternative here would be to go ahead with a breaking change to switch away from the option "prefix" style to just simply accepting the method pattern itself. That would mean that we would ultimately have less upkeep long term.

timtebeek commented 9 months ago

From a user perspective I think it would be easier to select from an enum dropdown than provide a methodPattern when you want to limit the results; we ought to be able to keep up with their SDK right? I imagine they're not churning out new feature flags often.

shanman190 commented 9 months ago

I agree with you there that there have not been new flag types added very quickly or often. As well as that using the option that accepts the prefix is definitely easier on the end user of the recipe as well.