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

feat!: Implement Requirement 1.1.3 #80

Merged
merged 15 commits into from
Mar 9, 2024
Merged

feat!: Implement Requirement 1.1.3 #80

merged 15 commits into from
Mar 9, 2024

Conversation

maxveldink
Copy link
Member

@maxveldink maxveldink commented Sep 25, 2023

This PR

Only the last four commits will be merged in with this PR. The others are waiting on #77 and #78.

  • Breaking: Renamed API.provider=(provider) to API.set_provider(provider, domain: nil) to better adhere to the specification. I decided to move away from the attr_writer naming convention as the domain parameter would be surprising. I would love others' thoughts on this.
  • Provider instances are now stored inside a provider hash in the config instance, with the default provider using nil as the key. The other option I tried here was to store @default_provider in addition to the hash, but this approach was simpler to the slight detriment of readability.
  • Refactored API.provider to receive domain as a keyword argument. If no domain is provided, it will return the default provider. This is backward compatible.
  • Implemented Requirement 1.1.3 from spec version 0.7.0.

Follow-up Tasks

In subsequent PRs, I'll continue on the flag evaluation specifications.

How to test

Open a console and play around with the set_provider method on the API. If no name is provided, the default provider will be replaced. If a name is provided, the provider will be registered using that name. If the same name is provided, the provider will be overridden.

@maxveldink maxveldink requested a review from a team as a code owner September 25, 2023 11:27
@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.39%. Comparing base (25680a4) to head (14b7791).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #80   +/-   ##
=======================================
  Coverage   99.39%   99.39%           
=======================================
  Files          12       12           
  Lines         164      166    +2     
=======================================
+ Hits          163      165    +2     
  Misses          1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maxveldink maxveldink requested review from josecolella and a team March 5, 2024 20:57
beeme1mr
beeme1mr previously approved these changes Mar 5, 2024
Signed-off-by: Max VelDink <[email protected]>
@maxveldink
Copy link
Member Author

@open-feature/sdk-ruby-maintainers Would love to get this merged in soon as well if anyone has some time for a review!

@beeme1mr beeme1mr changed the title feat: Implement Requirement 1.1.3 feat!: Implement Requirement 1.1.3 Mar 7, 2024
@beeme1mr
Copy link
Member

beeme1mr commented Mar 7, 2024

I updated the PR title to mark it as a breaking change.

@maxveldink maxveldink merged commit bc65e7a into open-feature:main Mar 9, 2024
11 checks passed
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.

3 participants