Betterment / test_track

Server app for the TestTrack multi-platform split-testing and feature-gating system
MIT License
119 stars 33 forks source link

Feature completion #100

Closed jmileham closed 5 years ago

jmileham commented 5 years ago

Summary

This adds a FeatureCompletion model and wires it into split visibility.

Notably this doesn't actually affect production behavior, and it's missing a piece before it would be safe. We also need to wire up modifications to the weighting registry for display to the same app (basically overriding displayed weightings to 100% false for feature gates on app versions that aren't in the feature-complete set).

Will mark up below.

/domain @Betterment/test_track_core /platform @samandmoore @smudge

nanda-prbot commented 5 years ago

Needs somebody from @Betterment/test_track_core to claim domain review Needs somebody from @samandmoore and @smudge to claim platform review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

smudge commented 5 years ago

Alright, so I think the README might need a bit of a terminology injection at this point, to include "feature completions" and also to make it clear how deciding a split (and setting "decided_at") compares to removing a split (which soft-deletes using "finished_at"), especially w.r.t. how mobile clients interpret all of that (since older clients can effectively circumvent both split decisions and soft deletions).

Happy to do that here, or wait until you finish wiring things up.

Also curious if you have any thoughts -- I am sort of questioning the "completion" terminology, to avoid confusion with the other two termination-sounding states/column values ("decided_at" and "finished_at"). What about "feature launch", to imply that it's the start of the feature's availability, not the end. It's silly but I legit had about 5 minutes of confusion trying to figure out the use case for telling apps when a feature split has been "completed" (i.e. finished?).

smudge commented 5 years ago

<< platform LGTM && domain TAFN

nanda-prbot commented 5 years ago

@jmileham needs to incorporate feedback from @smudge. Bump when done.

smudge commented 5 years ago

Did another pass! Thanks for talking me through to arriving at the missing hyphen in my mental model.

domain LGTM aside from the one spec case.

nanda-prbot commented 5 years ago

Approved! :dancers: :balloon: :taco:

jmileham commented 5 years ago

Note to self: there's a future feature that will add to/modify this behavior. Once we have "hard" assignments (future PR), hard assignments more recent than a relevant feature_completion will win over those feature completions.

smudge commented 5 years ago

Re-upping the LGTM on the two recent commits 👍