-
Notifications
You must be signed in to change notification settings - Fork 226
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
Remove some feature flags #5596
Conversation
Preview URL 🚀 : https://blurts-server-pr-5596-mgjlpikfea-uk.a.run.app |
"DiscountCouponNextThreeMonths", | ||
"LatestScanDateCsatSurvey", | ||
"AutomaticRemovalCsatSurvey", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if we could keep all the feature flags related to the CSAT surveys. We only recently landed a PR that extends the functionality of this CSAT PR #5586.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure: when you say "keep all the feature flags", in the context of this PR you only mean undoing the removal of AutomaticRemovalCsatSurvey
, right? I did that in 2cac1d1. Thanks for noticing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Yes, I specifically mean “keep all the feature flags related to the CSAT surveys”.
"DiscountCouponNextThreeMonths", | ||
"LatestScanDateCsatSurvey", | ||
"AutomaticRemovalCsatSurvey", | ||
"AdditionalRemovalStatuses", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is also a feature flag we would need to keep. We’ve recently disabled it with an issue related to the last opt-out removal. Even though, this issue will be addressed by PR #5591 the flag AdditionalRemovalStatuses
, it depends on the opt-out removal to be activated and to work correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted this in 82df2e2. Could you create a ticket that specifies when this flag is good to remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve created MNTOR-4048 for when we are ready to remove the flag AdditionalRemovalStatuses
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for cleaning up the feature flags.
82df2e2
to
c6d185b
Compare
Cleanup completed - database 'blurts-server-pr-5596' destroyed, cloud run service 'blurts-server-pr-5596' destroyed |
Removed the flags
ConfirmCancellation
,FirstDataBrokerRemovalFixedEmail
andAdditionalRemovalStatuses
, which were relatively easy to remove and have been on in production for a while now.Update: also removed
AutomaticRemovalCsatSurvey
,BreachEmailRedesign
,MonthlyReportFreeUser
andUpdatedEmailPreferencesOption
.