Betterment / test_track

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

Add app id seed #82

Closed effron closed 6 years ago

effron commented 6 years ago

Summary

To go along with this update to the test_track_rails_client https://github.com/Betterment/test_track_rails_client/pull/60, this adds a default "TestTrack" app and an "app_id" identifier type to the seeds file, so that consumer apps all don't have to create their own "app_id" identifier type.

For new apps, the seeds will run as part of rake db:setup, and for existing apps, you will have to run rake db:seed. It is designed to be idempotent, and accept any already existing apps with the name "TestTrack" or existing identifier types with the name "app_id".

/domain @Betterment/test_track_core /no-platform

nanda-prbot commented 6 years ago

@effron needs to request domain and platform reviewers, or explicitly opt out, e.g.:

/no-platform

effron commented 6 years ago

nanda?

nanda-prbot commented 6 years ago

Needs somebody from @Betterment/test_track_core to claim domain review

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

@myname << domain && platform

jmileham commented 6 years ago

Worth hitting it from a migration as well instead of requiring seeds to be run manually for existing testtrack installs?

samandmoore commented 6 years ago

<<domain

nanda-prbot commented 6 years ago

Needs @samandmoore 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

samandmoore commented 6 years ago

sure, we can throw a migration in. i suggested we just skip it because we can run the seed command one-off, but no biggie.

do we need to document anything here?

domain tafn

nanda-prbot commented 6 years ago

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

effron commented 6 years ago

I'm not sure if we need to document here, or in test_track_rails_client (or both?). maybe something about when to use app_id, and something about when you should lean on TestTrack.app_ab vs a normal test_track_visitor.test_track_ab

jmileham commented 6 years ago

If we skip the migration we gotta add an upgrading section to the ever growing readme is all.

On Tue, Jul 31, 2018 at 3:09 PM Sam Moore notifications@github.com wrote:

sure, we can throw a migration in. i suggested we just skip it because we can run the seed command one-off, but no biggie.

do we need to document anything here?

domain tafn

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/Betterment/test_track/pull/82#issuecomment-409333493, or mute the thread https://github.com/notifications/unsubscribe-auth/AACeq5YM6_eHTPIC0bKb57sOkmy1Clvnks5uMKtdgaJpZM4VoaBD .

samandmoore commented 6 years ago

Makes sense. I was just thinking we probably need to add a line about running seeds on initial setup anyway. On Tue, Jul 31, 2018 at 17:56 John Mileham notifications@github.com wrote:

If we skip the migration we gotta add an upgrading section to the ever growing readme is all.

On Tue, Jul 31, 2018 at 3:09 PM Sam Moore notifications@github.com wrote:

sure, we can throw a migration in. i suggested we just skip it because we can run the seed command one-off, but no biggie.

do we need to document anything here?

domain tafn

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <https://github.com/Betterment/test_track/pull/82#issuecomment-409333493 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AACeq5YM6_eHTPIC0bKb57sOkmy1Clvnks5uMKtdgaJpZM4VoaBD

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Betterment/test_track/pull/82#issuecomment-409380850, or mute the thread https://github.com/notifications/unsubscribe-auth/AAY4IuahjpPebU9Qd69UyCds0sEE0RAmks5uMNJ0gaJpZM4VoaBD .

effron commented 6 years ago

the initial setup recommends running rake:db:setup which will run seeds, so i'm nto sure we need to add explicit guidance. I added a migration, too

samandmoore commented 6 years ago

domainlgtm! nice

nanda-prbot commented 6 years ago

Approved! :sun_with_face: :star: :squirrel: