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.

Sign up for GitHub

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

Jump to bottom

ACC-749 x-gift-article improvements wrapped in a flag #579

Merged
merged 4 commits into from
Feb 12, 2021
Merged

ACC-749 x-gift-article improvements wrapped in a flag #579

merged 4 commits into from
Feb 12, 2021

Conversation

AniaMakes
Copy link
Contributor

@AniaMakes AniaMakes commented Jan 27, 2021 •
edited
Loading

This PR is #578 wrapped in a flag so that we can run an AB test.

Screenshot 2021-01-21 at 11 43 54
Screenshot 2021-01-21 at 11 27 27

This PR is one of many steps to get the AB test rolled out:

  • create a flag in flags API, with the status off
  • merge a PR in next-article that passes the flag status to x-gift-article - https://github.com/Financial-Times/next-article/pull/4117
  • merge this PR, release it, make sure the release propagates where needed
  • run a manual QA with flag on in toggler to make sure it all works as intended
  • turn the AB test on

@apaleslimghost apaleslimghost temporarily deployed to x-dash-x-gift-ab-improv-ulc2yl January 27, 2021 11:34 Inactive
@AniaMakes AniaMakes marked this pull request as ready for review January 28, 2021 17:17
@AniaMakes AniaMakes requested a review from a team as a code owner January 28, 2021 17:17
Base automatically changed from master to main February 8, 2021 13:05
thurstontye
thurstontye suggested changes Feb 8, 2021
View reviewed changes
Copy link
Contributor

@thurstontye thurstontye left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The A/B condition naming needs to be more consitant.

thurstontye
thurstontye previously approved these changes Feb 10, 2021
View reviewed changes
@thurstontye
Copy link
Contributor

@AniaMakes not essential but it might be worth adding an explanation of the flag in x-gift-article readme https://github.com/Financial-Times/x-dash/blob/main/components/x-gift-article/readme.md

@AniaMakes AniaMakes merged commit 7727c9c into main Feb 12, 2021
@AniaMakes AniaMakes deleted the x-gift-AB-improvements branch February 12, 2021 09:42
rowanbeentje pushed a commit that referenced this pull request Nov 24, 2021
@AniaMakes
Merge pull request #579 from Financial-Times/x-gift-AB-improvements
0107dc8 
ACC-749 x-gift-article improvements wrapped in a flag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oluoluoxenfree oluoluoxenfree oluoluoxenfree approved these changes

@thurstontye thurstontye thurstontye left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AniaMakes @thurstontye @oluoluoxenfree @apaleslimghost