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.

Sign up for GitHub

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

Jump to bottom

feat: add support for domains #805

Merged
merged 9 commits into from
Feb 20, 2024
Merged

feat: add support for domains #805

merged 9 commits into from
Feb 20, 2024

Conversation

beeme1mr
Copy link
Member

@beeme1mr beeme1mr commented Jan 31, 2024 •
edited
Loading

This PR

  • adds domain as a concept to the server and web SDK
  • adds a deprecation warning anywhere client name was exposed to users.
  • fixes an issue in the web SDK where context set on a domain before a provider is registered was not used.

Addresses

fixes #820 4aa9657

Notes

This change is based on this spec change. I tried to make it a non-breaking change but I may have missed an untested condition. Please carefully review to make sure I didn't miss anything.

Follow-up Tasks

  • Update the doc readme parser to support "domain".
  • Update the NestJS and React SDKS. We should consider making those a breaking change since they're sub 1.0.

@beeme1mr
rebrand named client to domain
3333680 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr beeme1mr requested a review from a team as a code owner January 31, 2024 22:59
@beeme1mr
fixed failing tests
4f52046 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr beeme1mr changed the title feat: rebrand named client to domain feat: add support for domains Feb 1, 2024
@toddbaert
Copy link
Member

Should we actually consider this a feat, considering it doesn't REALLY add new features? Maybe it's more of a chore? I guess you could argue enhanced clarity is some kind of feature?

@beeme1mr
Copy link
Member Author

beeme1mr commented Feb 1, 2024

I originally opened this as a refactor but we do add domain to event payloads. That's why I changed it to a feature. I'm good either way though. Whatever would be the most clear to users reading the release notes.

lukas-reining
lukas-reining approved these changes Feb 1, 2024
View reviewed changes
Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

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

Looks good, left one thought.

packages/client/src/client/open-feature-client.ts Show resolved Hide resolved
@toddbaert
fixup: missing domain assertions
3942ae6 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
Copy link
Member

@beeme1mr I found a few missing test assertions of domain where we were already asserting clientName, so I added them, but I can't see anything else. I did leave a couple minor suggestions above.

@toddbaert toddbaert self-requested a review February 2, 2024 18:06
beeme1mr and others added 3 commits February 8, 2024 13:06
@beeme1mr @toddbaert
Apply suggestions from code review
5d2f2fb 
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
updated readme based on feedback
6b5f035 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
Merge branch 'main' into convert-to-domain
1cb75f5 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
fix domain context pre set provider issue
4aa9657 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
updated readme based on feedback
91bf4d3 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
Copy link
Member Author

I've addressed all the feedback and included a bug fix. I'll merge this tomorrow unless I hear any objections. Thanks!

@beeme1mr beeme1mr added this pull request to the merge queue Feb 20, 2024
Merged via the queue into main with commit 98ba00a Feb 20, 2024
11 checks passed
@beeme1mr beeme1mr deleted the convert-to-domain branch February 20, 2024 16:35
This was referenced Feb 20, 2024
Merged
Merged
@openfeaturebot openfeaturebot mentioned this pull request Feb 20, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Feb 22, 2024
@openfeaturebot
chore(main): release core 0.0.26 (#823)
2fc38bd 
🤖 I have created a release *beep* *boop*
---


##
[0.0.26](core-v0.0.25...core-v0.0.26)
(2024-02-22)


### ✨ New Features

* add support for domains
([#805](#805))
([98ba00a](98ba00a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
Signed-off-by: openfeature-peer-update-bot <openfeature-peer-update-bot@openfeature.dev>
Co-authored-by: openfeature-peer-update-bot <openfeature-peer-update-bot@openfeature.dev>
toddbaert pushed a commit that referenced this pull request Feb 22, 2024
@openfeaturebot
chore(main): release server-sdk 1.12.0 (#806)
e3a7378 
🤖 I have created a release *beep* *boop*
---


##
[1.12.0](server-sdk-v1.11.0...server-sdk-v1.12.0)
(2024-02-22)


### ✨ New Features

* add support for domains
([#805](#805))
([98ba00a](98ba00a))


### 🧹 Chore

* **main:** release core 0.0.26
([#823](#823))
([2fc38bd](2fc38bd))


### 📚 Documentation

* add missing punctuation to readme
([4c73189](4c73189))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
toddbaert added a commit that referenced this pull request Feb 22, 2024
@openfeaturebot @toddbaert
chore(main): release web-sdk 0.4.13 (#807)
6034df4 
🤖 I have created a release *beep* *boop*
---


##
[0.4.13](web-sdk-v0.4.12...web-sdk-v0.4.13)
(2024-02-22)


### ✨ New Features

* add support for domains
([#805](#805))
([98ba00a](98ba00a))


### 🧹 Chore

* **main:** release core 0.0.26
([#823](#823))
([2fc38bd](2fc38bd))


### 📚 Documentation

* fix runsOn example on readme
([1096ae5](1096ae5))
* improved grammar, added punction to readme
([70bd33d](70bd33d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
This was referenced Mar 12, 2024
Closed
Closed
This was referenced Mar 15, 2024
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@federicobond federicobond federicobond left review comments

@toddbaert toddbaert toddbaert approved these changes

@lukas-reining lukas-reining lukas-reining approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] named context not working for web provider
4 participants
@beeme1mr @toddbaert @federicobond @lukas-reining