chore(deps): bump open-feature/go-sdk to v1.17.2 - #310
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the project’s OpenFeature integration to be compatible with github.com/open-feature/go-sdk v1.17.2, including removal of now-obsolete provider status handling introduced for older OpenFeature interfaces.
Changes:
- Bump
github.com/open-feature/go-sdkfrom v1.14.1 to v1.17.2 (and update related transitive deps ingo.mod/go.sum). - Remove
DevCycleProvider.Status()since the OpenFeature provider interface no longer includes it. - Update the close-related OpenFeature provider test to validate shutdown/closed behavior without calling
Status().
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| openfeature_provider.go | Removes the deprecated Status() method from DevCycleProvider after upgrading OpenFeature SDK. |
| openfeature_provider_test.go | Adjusts provider close/shutdown assertions to no longer rely on Status(). |
| go.mod | Updates direct/indirect module requirements for the OpenFeature SDK bump. |
| go.sum | Refreshes module checksums to match updated dependency graph. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Status expose the status of the provider | ||
| func (p DevCycleProvider) Status() openfeature.State { | ||
| if p.Client.closed() { | ||
| return openfeature.FatalState | ||
| } | ||
| return openfeature.ReadyState | ||
| } | ||
|
|
There was a problem hiding this comment.
Removing this functionality requires a major bump. Just keeping it around and marking deprecated is safer.
There was a problem hiding this comment.
I had Claude look into this, and I don't think we need to make it a major version, claude's research:
yea it's an exported method so I agree it's technically breaking. Worth noting the history though: Status() was part of the StateHandler interface (Init/Shutdown/Status) up to v1.13.0, and v1.14.0 dropped it. We added Status() in #280, which is the same commit that pinned v1.14.1, so it was dead on arrival: no released version of this SDK has shipped a go-sdk that calls it, and it was never documented for direct use.
happy to keep it with a // Deprecated: marker instead if you'd rather play it safe, it's only a few lines. I lean toward removing since it has never done anything, but your call.
There was a problem hiding this comment.
I would rather play it safe, just to avoid any potential version incompatibilities if someone didn't update their openfeature SDK to latest, etc.
It's a small chance - but better safe than sorry - especially because we don't know what customers may be using
There was a problem hiding this comment.
yea from what I can see this code was never executed because the version of the OF SDK required for using the provider never used this function, I really don't see a point in bumping a major version for that.
Summary
github.com/open-feature/go-sdkfrom v1.14.1 to v1.17.2DevCycleProvider.Status(), which is dead code as of go-sdk v1.16.0go build,go vet, andgo test ./...all passNotes
v1.16.0 (spec 0.8.0) made providers stateless and dropped
Status()fromopenfeature.FeatureProvider, so the SDK no longer calls it. It was added in #280 only to satisfy the old interface, is not in the README or any example, and the only callers were its own tests. It also ignoredinitialized()and returnedReadyStatebefore init completed. Consumers wanting provider state should useopenfeature.Client.State().TestOFDevCycleProvider_Closenow asserts viainternalFullClient.closed(), so coverage is unchanged.Transitive:
testifyv1.10.0 to v1.11.1 (forced by MVS), andgo.uber.org/mockis now a build-time indirect dep because go-sdk ships its mocks as non-test files.