Skip to content

[FSSDK-11551] Adding project config support for holdouts to go-sdk #415

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
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[FSSDK-11551] Adding project config support for holdouts to go-sdk #415

wants to merge 4 commits into from

Conversation

Mat001
Copy link
Contributor

@Mat001 Mat001 commented Aug 8, 2025

Update ProjectConfig to establish the flag's dependency on holdouts.

Jira: https://jira.sso.episerver.net/browse/FSSDK-11551

@Mat001 Mat001 self-assigned this Aug 8, 2025
@Mat001 Mat001 marked this pull request as ready for review August 12, 2025 17:19
Copilot
Copilot AI reviewed Aug 12, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for holdouts in the Go SDK by establishing flag dependencies on holdouts through the ProjectConfig. The change enables features to track and reference holdout IDs for proper experiment isolation and traffic allocation.

  • Adds HoldoutIDs field to the Feature struct and related entities
  • Updates the feature mapping logic to populate holdout IDs from datafile
  • Includes comprehensive test coverage for the new holdout functionality

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
pkg/entities/feature.go Adds HoldoutIDs field to the Feature struct
pkg/config/datafileprojectconfig/entities/entities.go Adds HoldoutIDs field to the FeatureFlag struct with JSON mapping
pkg/config/datafileprojectconfig/mappers/feature.go Updates MapFeatures function to populate holdout IDs from datafile
pkg/config/datafileprojectconfig/mappers/feature_test.go Adds test coverage for holdout ID mapping functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

muzahidul-opti
muzahidul-opti requested changes Aug 13, 2025
View reviewed changes
@Mat001 Mat001 requested a review from muzahidul-opti August 13, 2025 22:25
@Mat001
Copy link
Contributor Author

Mat001 commented Aug 14, 2025 •
edited
Loading

I added holdouts model.

I followed go & go-sdk idiomatic pattern so I created a new holdouts.go file and grouped holdout fields inside HoldoutMaps struct. go-sdk does similarly for other parts of sdk. Let me know if that is too much different from other sdk-s and we prefer to have everything inside project config. We could do that, just that then go-sdk will have this different that rest of the sdk because the rest of the sdk favors golang patterns specific to go versus consistency across sdk-s.

Here is comparison to Js:

  1. Structure:
    • JS: Holdout fields are directly on ProjectConfig (flat structure)
    • Go: Holdout fields are grouped in a HoldoutMaps struct (nested structure)
  2. Access:
    - JS: projectConfig.globalHoldouts (direct access)
    - Go: projectConfig.holdoutMaps.GlobalHoldouts (nested access)
  3. Function Location:
    - JS: getHoldoutsForFlag() is a standalone function that takes projectConfig
    - Go: GetHoldoutsForFlag() is also standalone but takes HoldoutMaps
====

One unrelated files is included (experiment bucketer service test) - just fixed formatting

@Mat001 Mat001 force-pushed the mpirnovar-holdouts-projconfig-fssdk-11551 branch from 2baa839 to 88e9b1a Compare August 14, 2025 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@muzahidul-opti muzahidul-opti Awaiting requested review from muzahidul-opti

Requested changes must be addressed to merge this pull request.

Assignees

@Mat001 Mat001

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Mat001 @muzahidul-opti