Skip to content

Feature flag refactoring (part 1)#4181

Merged
pcapriotti merged 34 commits intodevelopfrom
pcapriotti/feature-refactoring
Aug 12, 2024
Merged

Feature flag refactoring (part 1)#4181
pcapriotti merged 34 commits intodevelopfrom
pcapriotti/feature-refactoring

Conversation

@pcapriotti
Copy link
Contributor

@pcapriotti pcapriotti commented Aug 2, 2024

First stab at refactoring feature flags and removing code duplication.

Highlights of changes:

  • Improved naming slightly. Features types are now called Feature, LockableFeature and LockableFeaturePatch.
  • TTL fields were removed from all records. They remain in the API, but are always hardcoded to be unlimited.
  • Turned AllFeatures into an extensible record type.
  • Removed WithStatusBase barbie.
  • Deleted obsolete computeFeatureConfigForTeamUser.
  • Abstracted getFeature and setFeature.
  • Abstracted getAllTeamFeatures.

https://wearezeta.atlassian.net/browse/WPB-10323

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Aug 2, 2024
@pcapriotti pcapriotti force-pushed the pcapriotti/feature-refactoring branch from 1253d97 to fb2ba8b Compare August 5, 2024 11:05
@pcapriotti pcapriotti force-pushed the pcapriotti/user-features branch from 5030c22 to 0d7768d Compare August 6, 2024 12:20
@pcapriotti pcapriotti force-pushed the pcapriotti/feature-refactoring branch 2 times, most recently from e854929 to 3c485c4 Compare August 7, 2024 09:44
@MangoIV MangoIV force-pushed the pcapriotti/feature-refactoring branch 2 times, most recently from b6e5ffb to f1f92c3 Compare August 7, 2024 11:54
@MangoIV MangoIV marked this pull request as ready for review August 7, 2024 12:16
@MangoIV MangoIV force-pushed the pcapriotti/feature-refactoring branch from f0d7159 to c67a907 Compare August 7, 2024 12:17
@pcapriotti pcapriotti force-pushed the pcapriotti/feature-refactoring branch from c67a907 to 9a033f1 Compare August 7, 2024 12:19
@MangoIV MangoIV force-pushed the pcapriotti/user-features branch from 0d7768d to 7c5ee3c Compare August 7, 2024 12:20
@MangoIV MangoIV force-pushed the pcapriotti/feature-refactoring branch from 9a033f1 to a8d7ff9 Compare August 7, 2024 12:21
Base automatically changed from pcapriotti/user-features to develop August 7, 2024 13:14
@pcapriotti pcapriotti changed the title Feature flag refactoring Feature flag refactoring (part 1) Aug 7, 2024
@pcapriotti pcapriotti force-pushed the pcapriotti/feature-refactoring branch from a8d7ff9 to 550f357 Compare August 7, 2024 13:17
Copy link
Contributor

@MangoIV MangoIV left a comment

Choose a reason for hiding this comment

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

kind of a big one, was really nice working on it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

this is so nice

@pcapriotti pcapriotti force-pushed the pcapriotti/feature-refactoring branch from c23ceca to a49cdf2 Compare August 8, 2024 06:52
@pcapriotti pcapriotti mentioned this pull request Aug 8, 2024
2 tasks
@pcapriotti pcapriotti force-pushed the pcapriotti/feature-refactoring branch from 0bf9e91 to 1fa5fe1 Compare August 8, 2024 07:54
Comment on lines 48 to 62
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be possible with recursion on SList which reifies the type level list. You also need a csontraint on all elements, so perhaps smth like cpara_SList might work this time.

pcapriotti and others added 18 commits August 9, 2024 09:17
- rewrite boilerplate to NP combinators
- regeneate nix derivations
- format all
- make galley, wire-subsystems and wire-api compile
Also rename FeatureSingletonMlsMigration{Config}
Co-authored-by: Mango The Fourth <40720523+MangoIV@users.noreply.github.com>
Co-authored-by: Mango The Fourth <40720523+MangoIV@users.noreply.github.com>
Also get rid of LiftForF constraint, which required
`UndecidableSuperClasses`.
Also remove unnecessary parenthesis.
@pcapriotti pcapriotti force-pushed the pcapriotti/feature-refactoring branch from 1fa5fe1 to ca74ec3 Compare August 9, 2024 07:24
Copy link
Contributor

@elland elland left a comment

Choose a reason for hiding this comment

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

If @MangoIV has no pending comments, LGTM.

dbFeatureTTL,
dbFeatureConfig,
dbFeatureModConfig,
WithStatus,
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

@pcapriotti pcapriotti merged commit cdeeb0b into develop Aug 12, 2024
@pcapriotti pcapriotti deleted the pcapriotti/feature-refactoring branch August 12, 2024 08:45
@echoes-hq echoes-hq bot added echoes: technical-roadmap/technical-debt More specific category, to highlight Technical Debt being tackled. echoes: technical-roadmap/throughput More specific category, to highlight task aiming at improving the development velocity and effici... labels Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

echoes: technical-roadmap/technical-debt More specific category, to highlight Technical Debt being tackled. echoes: technical-roadmap/throughput More specific category, to highlight task aiming at improving the development velocity and effici... ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants