Skip to content

feat(Flag Dependency): Evaluate flags that depend on other flags in local evaluation - #70

Closed
matthewelwell wants to merge 0 commit into
mainfrom
linear/sdk-8-enable-flag-dependency-in-the-rust-server-side-sdk-7f71
Closed

matthewelwell wants to merge 0 commit into
mainfrom
linear/sdk-8-enable-flag-dependency-in-the-rust-server-side-sdk-7f71

Conversation

@matthewelwell

@matthewelwell matthewelwell commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Local evaluation ignores segment overrides that depend on another flag, so it disagrees with remote evaluation.

Closes SDK-8. Depends on Flagsmith/flagsmith-rust-flag-engine#42; merge once it's released as 0.7.0.

Changes

  • Overrides on $.flags.<feature>.enabled never matched; they now apply to identity and environment flags.
  • Environment flags ignored every segment; they now get overrides from segments that need no identity (e.g. $.environment.name), as in the Java SDK.
  • The SDK spec runs v0.7.0, including its local evaluation scenarios.

🤖 Generated with Claude Code

@matthewelwell
matthewelwell requested a review from a team as a code owner October 21, 2026 18:06
@matthewelwell
matthewelwell requested review from emyller and removed request for a team October 21, 2026 18:06
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 8a53216a-10f8-4802-bdd0-dda00e905d6d
Walkthrough

Both Cargo manifests now pin flagsmith-flag-engine to the same Git revision. A new environment fixture and parameterised integration test check whether the dependent identity flag receives its segment override when the prerequisite flag is enabled, and retains its default state and value when it is disabled.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: Moderate · previous review 45fdf

The dependency is suitable for interim validation, but this PR is not ready to merge until engine 0.7.0 is available and both manifests use it.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 86f3538c-fd98-4876-934a-8f967a33db37
📥 Commits

Reviewing files that changed from the base of the PR and between 1d1a2bb and 45fdfc0.

⛔ Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing: 4
  • Cargo.toml
  • sdk-spec/Cargo.toml
  • tests/fixtures/mod.rs
  • tests/integration_test.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Comment thread Cargo.toml Outdated
log = "0.4"
flume = "0.10.14"
flagsmith-flag-engine = "0.6"
flagsmith-flag-engine = { git = "https://github.com/Flagsmith/flagsmith-rust-flag-engine", rev = "6d892ee7cf44eb5860899e925aa2433de4356c2f" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Replace the Git dependency before merge.

The release manifest must use flagsmith-flag-engine = "0.7" when engine 0.7.0 is available on crates.io. This Git revision is valid for interim validation but does not satisfy the stated release contract. Update this entry and the matching sdk-spec/Cargo.toml entry before merge.

@khvn26

khvn26 commented Oct 30, 2026

Copy link
Copy Markdown
Member

Needs rebasing.

@emyller emyller closed this Oct 30, 2026
@emyller
emyller force-pushed the linear/sdk-8-enable-flag-dependency-in-the-rust-server-side-sdk-7f71 branch from 3576b8e to 4bf93a4 Compare October 30, 2026 16:22
@emyller
emyller deleted the linear/sdk-8-enable-flag-dependency-in-the-rust-server-side-sdk-7f71 branch October 30, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants