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.

Sign up for GitHub

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

Jump to bottom

[SERF-3458] Introduce Statsig package #121

Merged
merged 2 commits into from
Jul 26, 2024
Merged

[SERF-3458] Introduce Statsig package #121

merged 2 commits into from
Jul 26, 2024

Conversation

laynax
Copy link
Contributor

@laynax laynax commented Jul 23, 2024 •
edited
Loading

Description

This PR introduces Statsig to leverage the functionality of experimentation and feature flag.
It includes:

  • A new statsig package along with its configuration.
  • Defined ToContext and Extract for both experiment and featureFlag.
  • Introduced middlewares and interceptors to inject experiment and featureFlag into the context to be used in middleware.
  • All include unit tests

Testing considerations

Tested on development

Checklist

  • Prefixed the PR title with the JIRA ticket code
  • Performed simple, atomic commits with [good commit messages][commit messages]
  • Verified that the commit history is linear and commits are squashed as necessary
  • Thoroughly tested the changes in development and/or staging
  • Updated the README.md as necessary

Related links

@laynax laynax force-pushed the laynax/SERF-3458/fflag branch 3 times, most recently from 09d8943 to 1b55b44 Compare July 24, 2024 07:35
@laynax laynax changed the title WIP [SERF-3458] Introduce Statsig package Jul 24, 2024
@laynax laynax marked this pull request as ready for review July 24, 2024 13:23
@laynax laynax requested a review from a team as a code owner July 24, 2024 13:23
@laynax laynax requested a review from Neurostep July 24, 2024 13:23
fotos
fotos reviewed Jul 24, 2024
View reviewed changes
Copy link
Contributor

@fotos fotos left a comment

Choose a reason for hiding this comment

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

Did a quick pass and left a few remarks. 💬

pkg/context/statsig/context.go Outdated Show resolved Hide resolved
pkg/context/statsig/context.go Outdated Show resolved Hide resolved
pkg/context/statsig/context.go Outdated Show resolved Hide resolved
pkg/interceptors/statsig.go Outdated Show resolved Hide resolved
pkg/interceptors/statsig.go Outdated Show resolved Hide resolved
pkg/interceptors/statsig.go Outdated Show resolved Hide resolved
pkg/statsig/config.go Outdated Show resolved Hide resolved
laynax
laynax commented Jul 24, 2024
View reviewed changes
Copy link
Contributor Author

@laynax laynax left a comment

Choose a reason for hiding this comment

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

@fotos Nice catches 👍
Fixed the typos and added 2 new test files ⛏️

fotos
fotos previously approved these changes Jul 24, 2024
View reviewed changes
Copy link
Contributor

@fotos fotos left a comment

Choose a reason for hiding this comment

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

👍 LGTM 🚀

Approved the PR – left 2 more (nit) remarks.

pkg/interceptors/request_id_test.go Outdated Show resolved Hide resolved
pkg/context/statsig/context.go Outdated Show resolved Hide resolved
@laynax laynax force-pushed the laynax/SERF-3458/fflag branch 3 times, most recently from a6826fb to a1158a5 Compare July 25, 2024 09:17
@laynax laynax requested a review from fotos July 25, 2024 11:40
@laynax laynax force-pushed the laynax/SERF-3458/fflag branch 2 times, most recently from 8cd9bbb to b7c05df Compare July 26, 2024 08:28
fotos
fotos previously approved these changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@fotos fotos left a comment •
edited
Loading

Choose a reason for hiding this comment

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

👍 LGTM 🚀

Does the documentation need an update too?

@laynax laynax force-pushed the laynax/SERF-3458/fflag branch 4 times, most recently from f777003 to 4c958a6 Compare July 26, 2024 11:00
fotos
fotos previously approved these changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@fotos fotos left a comment

Choose a reason for hiding this comment

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

👍 LGTM 🚀

@laynax laynax requested a review from fotos July 26, 2024 11:44
fotos
fotos approved these changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@fotos fotos left a comment

Choose a reason for hiding this comment

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

👍 LGTM 🚀

@laynax laynax merged commit 1a9fa72 into main Jul 26, 2024
5 of 6 checks passed
@laynax laynax deleted the laynax/SERF-3458/fflag branch July 26, 2024 15:32
@fotos fotos mentioned this pull request Jul 26, 2024
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fotos fotos fotos approved these changes

@Neurostep Neurostep Awaiting requested review from Neurostep Neurostep is a code owner automatically assigned from scribd/service-foundations

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@laynax @fotos