Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Brave Talk study to be in Release #119

Merged
merged 1 commit into from
Sep 22, 2021

Conversation

bsclifton
Copy link
Member

Per today's triage (Sep 7th), here's the uplift for Talk to RELEASE channel

cc: @jsecretan for the go-ahead. Was uplifted / tested against staging here:
#93

@bsclifton bsclifton requested review from jsecretan and a team September 7, 2021 18:28
@bsclifton bsclifton self-assigned this Sep 7, 2021
Copy link

@rebron rebron left a comment

Choose a reason for hiding this comment

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

Looks good. ++

@kjozwiak kjozwiak merged commit 10506b8 into production Sep 22, 2021
@kjozwiak kjozwiak deleted the bsc-brave-talk-to-release branch September 22, 2021 15:57
@@ -913,7 +913,7 @@
}
],
"filter": {
"channel": ["BETA", "NIGHTLY"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

typically we should specify min_version, to not enable it for old clients @bsclifton @rebron

Copy link
Collaborator

Choose a reason for hiding this comment

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

also @kjozwiak

Copy link
Member

Choose a reason for hiding this comment

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

@iefremov can we get this fixed or is it too late at this point? What would happen if a client that ended up pulling the above study doesn't meet the new requirements. Would it simply be removed? Assuming it wouldn't cause any issues for users that meet the requirements and have the study?

I'll make sure we have a min_version next time 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the heads up - will do that going forward. No action needed for this, AFAIK. But they would be shown as being in an experiment which isn't applicable

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.

4 participants