spotify / confidence-sdk-go

Apache License 2.0
0 stars 1 forks source link

README Example Outdated: Incorrect Package Reference for NewAPIConfig #56

Open moesy opened 1 month ago

moesy commented 1 month ago

The README example for using the Confidence SDK with OpenFeature appears to be out of date. Specifically, the lines:

import (
    "github.com/open-feature/go-sdk/openfeature"
    confidence "github.com/spotify/confidence-sdk-go/pkg/provider"
)
....

provider, err := confidence.NewFlagProvider(confidence.NewAPIConfig("clientSecret"))

is incorrect because the NewAPIConfig function is not part of the github.com/spotify/confidence-sdk-go/pkg/provider package. Instead, it is located in the github.com/spotify/confidence-sdk-go/pkg/confidence package.

nicklasl commented 1 month ago

Thanks for taking the time to report this, we'll take care of it right away 👍

moesy commented 1 month ago

Thank you @nicklasl for the quick response!

The proposed solution works however your variable names collide with the package names so the go linters are going to complain a bit.

I would recommend either changing

confidence := confidence.NewConfidenceBuilder().SetAPIConfig(confidence.APIConfig{APIKey: "clientSecret"}).Build()
provider := provider.NewFlagProvider(confidence)

to something like:

confidenceApp := confidence.NewConfidenceBuilder().SetAPIConfig(confidence.APIConfig{APIKey: "clientSecret"}).Build()
confidenceProvider := provider.NewFlagProvider(confidenceApp)

The import aliases e.g: confidence "github.com/spotify/confidence-sdk-go/pkg/confidence" also seem redundant since the package names are the same. if you wanted to keep your variable names as confidence and provider you should change the aliases to another name.

nicklasl commented 1 month ago

Ahh! thanks!

I'm thinking, since this is a readme, I may keep the import aliases to signal where the functions come from.

moesy commented 1 month ago

Keeping the alias sounds fine for readability - thanks again! The variable names assigned later in the example should change then to stop the name collisions. Other than that I can confirm everything works as intended.