Skip to content

feat: Init web-sdk go-feature-flag #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 29 commits into from
Jul 31, 2023

Conversation

thomaspoignant
Copy link
Member

This PR

  • 1st version of the web-sdk for GO Feature Flag

Related Issues

Fixes thomaspoignant/go-feature-flag#662

Signed-off-by: Thomas Poignant <[email protected]>
@thomaspoignant thomaspoignant changed the title Init web-sdk go-feature-flag feat: Init web-sdk go-feature-flag Jul 19, 2023
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
@toddbaert
Copy link
Member

@thomaspoignant super cool! I will give this a full review later today!

@thomaspoignant
Copy link
Member Author

@toddbaert I've addressed all your comments, including using fetch rather than axios.
If you don't mind doing another review it would be awesome.

I think we should wait the release of a stable SDK to be available before merging this PR.

Signed-off-by: Thomas Poignant <[email protected]>
@toddbaert
Copy link
Member

Sorry @thomaspoignant I will re-review this first thing tomorrow

@thomaspoignant
Copy link
Member Author

Sorry @thomaspoignant I will re-review this first thing tomorrow

No rush, no worries

Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomaspoignant looks great to me! Only minor nits that are opinions. Please be careful resolving conflicts in the .release-please-manifest 😅

I plan on releasing our first non-experimental web-sdk on Monday, with this change: open-feature/js-sdk#507 (comment). I'm waiting until then to give people a few days to update the providers impacted by the change.

You can wait until then if you'd like to set a proper peer for this.

Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, nice job @thomaspoignant! We should extend the playground to support client-side GOFF once this has been published.

@beeme1mr
Copy link
Member

@toddbaert, it looks like the CI is failing because of flagd. I'm assuming the NX affecting command detected so many components because the root package json was modified, right? Either way, we should unblock Thomas.

@thomaspoignant
Copy link
Member Author

Awesome, nice job @thomaspoignant! We should extend the playground to support client-side GOFF once this has been published.

Sure I will look at it as soon as we merge it.

@toddbaert
Copy link
Member

toddbaert commented Jul 31, 2023

@toddbaert, it looks like the CI is failing because of flagd. I'm assuming the NX affecting command detected so many components because the root package json was modified, right? Either way, we should unblock Thomas.

@beeme1mr ya that's why all the packages were rebuilt, but more specifically there's a race condition that happens sometimes when git attempts to pull sub-modules. It's rare, but annoying:

error: could not lock config file /home/runner/work/js-sdk-contrib/js-sdk-contrib/.git/modules/libs/providers/flagd-web/schemas/config: File exists
fatal: could not set 'core.worktree' to '../../../../../../libs/providers/flagd-web/schemas'
Warning: run-commands command "git submodule update --init --recursive" exited with non-zero status code

I re-ran the suite, but now it's failing on some GoFF specific tests. I will see what can be done about the submodule race.

@thomaspoignant
Copy link
Member Author

I re-ran the suite, but now it's failing on some GoFF specific tests. I will see what can be done about the submodule race.

I can have a look at those ones.

@thomaspoignant thomaspoignant merged commit d1ff1ec into open-feature:main Jul 31, 2023
@thomaspoignant thomaspoignant deleted the add-web-goff branch July 31, 2023 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(feature) Implement Open-feature web provider.
4 participants