Skip to content

Conversation

@nmayorsplit
Copy link

SPLIT DAEMON

What did you accomplish?

  • Added impression properties in treatments

How do we test the changes introduced in this PR?

Extra Notes

@nmayorsplit nmayorsplit requested a review from a team January 20, 2026 19:01
return dtos.NewFallbackTreatmentCalculatorImp(&fallbackTreatmentConf)
}

func serializeProperties(opts *dtos.EvaluationOptions) string {

Choose a reason for hiding this comment

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

I think it’s already covered, but just in case, we should verify that when no evaluation options or properties are provided (or when the provided properties are invalid), the properties field is omitted from the backend payload (same as the SDKs).

Also, I think the impression sent to the thin client should not include the properties field when serializeProperties returns an empty string. Otherwise (not 100% sure) it might break compatibility with thin-client versions that don’t handle that field.

Copy link

@EmilianoSanchez EmilianoSanchez left a comment

Choose a reason for hiding this comment

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

LGTM, just a comment to double-check.

@nmayorsplit nmayorsplit merged commit a4b303e into FME-8315 Jan 21, 2026
4 checks passed
@nmayorsplit nmayorsplit deleted the FME-12498 branch January 21, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants