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

Add FeatureSwitchProvider and use it for 'show Gu suppliers' functionality #149

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bryophyta
Copy link
Contributor

@bryophyta bryophyta commented Feb 10, 2025

What does this change?

Adds a FeatureSwitchProvider object which contains config for feature switches, and exposes a limited set of public methods that will hopefully make feature switch use intuitive and clear, i.e. it currently just exposes lists of switches, which have an isOn method.

Possibly over-engineering given that we've only got a single case at the moment. I lean towards thinking it's worthwhile because it will hopefully help us stay disciplined when adding this kind of logic, but open to other perspectives.

Also refactors the client side filtering out of Gu* suppliers so that it consumes the same switch, rather than consuming a list of suppliers to filter out. There's a bit more code duplication here but it seems overall simpler to have the contract between the client and server fit the same model as the switch mechanism here.

How to test

How can we measure success?

Have we considered potential risks?

Images

Accessibility

@bryophyta bryophyta force-pushed the pf/feature-switch-provider branch from 428b407 to 9b0a71e Compare February 10, 2025 16:51
@bryophyta bryophyta changed the title Pf/feature switch provider Add FeatureSwitchProvider Feb 13, 2025
@bryophyta bryophyta force-pushed the pf/feature-switch-provider branch from 9b0a71e to d35b29b Compare February 13, 2025 17:15
@bryophyta bryophyta changed the title Add FeatureSwitchProvider Add FeatureSwitchProvider and use it for 'show Gu suppliers' functionality Feb 13, 2025
@bryophyta bryophyta marked this pull request as ready for review February 13, 2025 17:29
@bryophyta bryophyta requested a review from a team as a code owner February 13, 2025 17:29
private val safeState: SwitchState
) {
def isOn: Boolean =
safeState == On // currently we're only using safeState to determine state
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the idea is that at a later stage we'd turn FeatureSwitchProvider into a class or something similar, which could then take a set of overrides which would determine isOn for a given switch.

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.

2 participants