Betterment / test_track

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

new versions of app_visitor_configs and app_identifier_visitor_configs #148

Closed samandmoore closed 3 years ago

samandmoore commented 3 years ago

Summary

new versions of app_visitor_configs and app_identifier_visitor_configs

these new endpoints will be used in by our newer versions of the mobile clients.

Other Information

/domain @Betterment/test_track_core /no-platform /cc @mseijas @ClaireDavis @btrautmann

nanda-prbot commented 3 years ago

@samandmoore needs to request platform review, or explicitly opt out, e.g.:

/no-platform

HOW TO: Request Reviewers

jmileham commented 3 years ago

I like introducing a new API version (v3) if we're adding a new shape. Will be easier to shim into deserialization code in clients if all the new stuff is named V3. Also if we have high confidence about the shape we'll need for other clients, we could def cut the endpoints now so it's a smaller lift to upgrade them later (or add new clients in new languages that get to benefit from the better shape off the bat).

jmileham commented 3 years ago

<< domain tafn

nanda-prbot commented 3 years ago

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

HOW TO: Resolve Feedback

samandmoore commented 3 years ago

I like introducing a new API version (v3) if we're adding a new shape. Will be easier to shim into deserialization code in clients if all the new stuff is named V3. Also if we have high confidence about the shape we'll need for other clients, we could def cut the endpoints now so it's a smaller lift to upgrade them later (or add new clients in new languages that get to benefit from the better shape off the bat).

we already have 1 thing in the api/v3 namespace which is a split_registry endpoint nested under builds. it’s what’s used by the current version of the rails client. and it uses a split registry format that’s newer than v2 but not the same as what i’m doing for these 2 endpoints. so does that mean we should go to v4?

i’d expect that we’d wanna have a v4 endpoint for /builds/:timestamp/split_registry for the rails client that uses this new format eventually, but i was thinking of not adding that until we wanna use it. there's no urgency to change the serialization format for the rails client since it's happily consuming the map-like approach

samandmoore commented 3 years ago

bump er ooooo

nanda-prbot commented 3 years ago

Needs @jmileham to provide domain review

When you finish a round of review, be sure to say you've finished or sign off on the PR, e.g.:

TAFN or DomainLGTM

If you're too busy to review, unclaim the PR, e.g.:

@myname >> domain

HOW TO: Give Feedback

jmileham commented 3 years ago

Domain lgtm!