Closed jmileham closed 5 years ago
Needs somebody from @Betterment/test_track_core to claim domain review Needs somebody from @JonLz 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 << domain platform
Needs somebody from @JonLz 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
nanda
Needs somebody from @JonLz to claim domain review
Use the shovel operator to claim, e.g.:
@myname << domain && platform
@smudge >> domain platform <<domain platform
Needs somebody from @JonLz to claim domain review
Use the shovel operator to claim, e.g.:
@myname << domain && platform
domaintafn platformlgtm -- but i think you've got one typo in there and i can't understand why the test isn't failing...
Needs somebody from @JonLz to claim domain review
Use the shovel operator to claim, e.g.:
@myname << domain && platform
ivars fre ftw
On Mon, Apr 15, 2019 at 6:09 PM Sam Moore notifications@github.com wrote:
@samandmoore commented on this pull request.
In app/views/api/v1/app_identifier_visitor_configs/show.json.jbuilder https://github.com/Betterment/test_track/pull/111#discussion_r275563703:
@@ -0,0 +1,4 @@ +json.partial! 'api/v1/app_visitor_configs/show',
- active_splits: @active_splits,
- visitor_id: @visitor_id,
seems like this line should be blowing up because we don't assign @visitor_id
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Betterment/test_track/pull/111#pullrequestreview-226904728, or mute the thread https://github.com/notifications/unsubscribe-auth/AACeq94RO62iFaP5yczXyyMDdAjL3vtaks5vhPisgaJpZM4cwf0F .
domainlgtm
Needs somebody from @JonLz to claim domain review
Use the shovel operator to claim, e.g.:
@myname << domain && platform
<< domain lgtm
Approved! :nail_care: :nut_and_bolt: :guitar:
@samandmoore - I went to town on validating the URL params. You now must bring an ISO date with second or millisecond precision or we'll return you a 422 with an error bag, which should be helpful to client library devs. Notably an app that can't be found by name will 404, not 422.
Here's the new commit
Re-LG please? It's a substantial diff.
small comment, otherwise domainlgtm
Summary
Provides endpoints for clients in the field to filter assignments and do contextual variant knockouts.
I believe this will be the last TT PR before MVP of mobile extensions is done server side.
Other Information
Per usual there's a bunch of commits in here that are not relevant due to stacked PRs. I'll call out the real diff below.
/domain @Betterment/test_track_core /domain @JonLz for mobile integration needs - this would be what you'd be implementing to client side. /platform @samandmoore @smudge