open-feature / go-sdk

Go SDK for OpenFeature
https://openfeature.dev
Apache License 2.0
126 stars 30 forks source link

feat: Move all files from pkg/openfeature to openfeature. #232

Closed paddycarver closed 9 months ago

paddycarver commented 10 months ago

This PR

Move the package from being github.com/open-feature/go-sdk/pkg/openfeature, which has an unnecessary pkg in the import path, to github.com/open-feature/go-sdk/openfeature, without the unnecessary "pkg" in the import path.

The existing github.com/open-feature/go-sdk/pkg/openfeature package is now a compatibility shell; exported types, constants, variables, and functions are now aliased to the new github.com/open-feature/go-sdk/openfeature equivalents in a way that will pass equality checks, and types are interchangeable between both packages. No logic or tests live in the package anymore, only the exported identifiers, which call through to the new package for behavior.

The memprovider package, similarly, has moved.

Related Issues

Fixes #227

How to test

Any consumers of the package that already exist should be able to transparently use this version (go mod edit -replace github.com/open-feature/go-sdk=github.com/paddycarver/open-feature@no-pkg) with no code changes or changes in behavior.

codecov[bot] commented 10 months ago

Codecov Report

Attention: 196 lines in your changes are missing coverage. Please review.

Comparison is base (34fb9d9) 81.61% compared to head (1098f60) 81.61%.

Files Patch % Lines
openfeature/client.go 76.76% 89 Missing and 10 partials :warning:
openfeature/memprovider/in_memory_provider.go 75.59% 26 Missing and 5 partials :warning:
openfeature/resolution_error.go 18.91% 30 Missing :warning:
openfeature/hooks.go 29.03% 22 Missing :warning:
openfeature/provider.go 65.38% 9 Missing :warning:
openfeature/evaluation_context.go 82.60% 4 Missing :warning:
openfeature/openfeature.go 97.67% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #232 +/- ## ======================================= Coverage 81.61% 81.61% ======================================= Files 10 10 Lines 1142 1142 ======================================= Hits 932 932 Misses 192 192 Partials 18 18 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Kavindu-Dodan commented 10 months ago

@paddycarver thank you for the contribution and I like the approach proposed through this PR (compared to #233 with root level content)

Could you please sign off your commit ? This guide should help you - https://github.com/open-feature/go-sdk/pull/232/checks?check_run_id=18694944964

paddycarver commented 10 months ago

Should be done. :)

toddbaert commented 10 months ago

@Kavindu-Dodan @paddycarver thanks so much for this change. I agreed very much with @craigpastro that the imports were not ideal, and I had already accepted that this would be a breaking change, so this was a welcome surprise!

There's a ! in the title. This implies this is breaking, which I believe it is not (I don't consider deprecations breaking). Since everything should work transparently with the exception of compiler warnings around deprecations, I guess we can remove the !? Otherwise our release automation will try to increment the package version to v2.0.0.

AlexsJones commented 10 months ago

What problem does this solve? It's good practice to use pkg in golang

If you want to improve usability I would suggest remove the last part of the slug github.com/open-feature/go-sdk/pkg/openfeature

beeme1mr commented 10 months ago

@AlexsJones, the reasoning was discussed in the linked issue. @paddycarver was able to introduce this change in a nonbreaking way.

AlexsJones commented 10 months ago

If we can make the change without breaking things 👍