Skip to content
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

Add Feature Flags support #29

Merged
merged 13 commits into from
Jun 8, 2021
Merged

Add Feature Flags support #29

merged 13 commits into from
Jun 8, 2021

Conversation

yakkomajuri
Copy link
Contributor

@yakkomajuri yakkomajuri commented Jun 7, 2021

Dear Reviewer

Given prettier was run here, feature-flags.js and test.js are the places to focus most of the energy on.

What this does

  1. Adds support for feature flags
  2. Make everything prettier

What this does not do

  1. Refactor the existing codebase - I was originally doing this but decided to leave this up to a separate PR. Things like consolidating all the requests, etc.

How the feature flags work

This follows the posthog-python implementation almost exactly.

Follow-up work

  1. Documentation
  2. Writing up a feature flags spec - indeed the posthog-python code somewhat serves as this, but we need a place that at least tells us how to correctly calculate simple flags so we don't end up with diverging implementations. I spent some time playing around with crypto to get this right.
  3. Refactor

@yakkomajuri yakkomajuri requested a review from Twixes June 7, 2021 18:08
@Twixes Twixes mentioned this pull request Jun 8, 2021
Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

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

Please please never mix code formatting changes with behavior changes. I made #30 to make this reviewable. Now this PR only includes material updates to the code.
A couple of comments besides that but overall this looks reasonable.

feature-flags.js Outdated Show resolved Hide resolved
index.d.ts Show resolved Hide resolved
@Twixes Twixes linked an issue Jun 8, 2021 that may be closed by this pull request
@yakkomajuri
Copy link
Contributor Author

@Twixes yeah I do apologize for the prettier changes coming in this PR

@yakkomajuri
Copy link
Contributor Author

Ready to go

@yakkomajuri
Copy link
Contributor Author

Also writing docs now

@Twixes Twixes added the bump minor Bump minor version when this PR gets merged label Jun 8, 2021
Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

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

Letsgooo

@Twixes Twixes merged commit 85aab5c into master Jun 8, 2021
@Twixes Twixes deleted the feature-flags branch June 8, 2021 13:19
@weyert
Copy link

weyert commented Jun 8, 2021

Awesome 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor Bump minor version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for feature flags
3 participants