-
Notifications
You must be signed in to change notification settings - Fork 325
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
[WPB-15151] eliminate legahold test redundancy #4386
Conversation
3ff6ea0
to
aa7cfa2
Compare
-- | ||
-- This flag allows to make tests run through both configurations with minimal adjustment. | ||
data ImplicitConsent = ImplicitConsent | ExplicitConsent | ||
deriving (Eq, Show, Generic) |
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.
Where does this need Generic
? 🤔
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.
for the test collection machinery
integration/test/Test/LegalHold.hs
Outdated
setLHFeatureConfigForServer ImplicitConsent app = | ||
-- we could do `setField "settings.featureFlags.legalhold" | ||
-- "whitelist-teams-and-implicit-consent"`, but this is already the default. | ||
-- TODO: make this an assertion, not a comment! |
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.
Can't this easily be done now?
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.
right. missed that spot, thanks!
Co-authored-by: Sven Tennie <[email protected]>
... to guard against accidental change.
thanks, sven! the CI failures in 64193e7 seem unrelated? |
More ❄️ 😬 |
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.
IMHO we can merge this as soon as the CI is 🟢
Checklist
changelog.d