-
Notifications
You must be signed in to change notification settings - Fork 22
Adds Flagsmith adapter #103
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
base: main
Are you sure you want to change the base?
Conversation
@tiagoapolo is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contributions so far! I left a bit of early guidance
@@ -0,0 +1,64 @@ | |||
# Flags SDK - Flagsmith Provider | |||
|
|||
A provider adapter for the Flags SDK that integrates with [Flagsmith](https://flagsmith.com/), allowing you to use Flagsmith's feature flags and remote configuration in your application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you work for Flagsmith? If we merge this PR we'd also put up some actual docs on flags-sdk.dev. Would you be wiling to link to our Flags SDK and those docs from https://docs.flagsmith.com?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @dferber90, just jumping in here since Tiago is on vacation - yes, Tiago works for Flagsmith.
In answer to your question, for sure, we'd be happy to link to your docs from https://docs.flagsmith.com - would you be happy with adding reciprocal backlink(s) too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! We'd add you to https://flags-sdk.dev/providers and create a docs page for FlagSmith
47974ab
to
62c9498
Compare
@dferber90 could you take one more look at my PR ? |
@tiagoapolo I‘m out of office this week, but @AAorris will take over leading adapter work and help you get this finalized, listed and released 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR! Please consider the following input:
Strongly recommended:
- Support identity through
entities
- Optimize performance for multiple features in one request
Recommended:
- Add origin to link flags to your customers' dashboards
Co-authored-by: Aaron Morris <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The progress here is looking good!
It looked ready enough while reviewing that I started to explore an example app using the adapter. However, I found several issues while testing in an app.
To continue the review with me, please develop an example that uses the adapter based on an existing one
- Standard templates: https://github.com/vercel/examples/tree/main/flags-sdk
- The slim template I branched for testing with Flagsmith
You can build, test, and deploy the demo using your local adapter code by following these instructions
Please share a link to the example app with your next review request. I look forward to it!
@AAorris in this branch you have my example running https://github.com/tiagoapolo/examples/tree/flagsmith-adapter. Also it's deployed in vercel at: https://flags-sdk-template-ff5v5lyuk-tiagos-projects-e9d5728f.vercel.app/ flags_compressed.mov |
Hi @tiagoapolo, thanks for your work and sorry for the delay. I'm back and will be reviewing this week if possible. I don't see your code pushed up to the branch, could you make sure it's available for me to reproduce? It would be great if the example shows how to replicate the flags in the same way as other examples:
|
This pull request introduces a new provider adapter for the Flags SDK that integrates with Flagsmith. The changes include adding configuration files, updating dependencies, and creating the necessary code for the adapter.
Flagsmith documentation: https://docs.flagsmith.com/clients/next-ssr
Usage