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

fix: various edge cases in targeting #1041

Merged
merged 12 commits into from
Dec 5, 2023
Merged

Conversation

toddbaert
Copy link
Member

@toddbaert toddbaert commented Nov 30, 2023

While discussing this, @beeme1mr and I realized it was worth adding a bit more precision here. Please let me know what you folks think, particularly @Kavindu-Dodan and @bacherfl who helped implement some of these.

The main reason this is important, is I will be creating some "negative" gherkin tests to make sure bad inputs and mis-configurations behave consistently across all flagd engine implementations.

Update: new gherkin is here.

Copy link

netlify bot commented Nov 30, 2023

Deploy Preview for polite-licorice-3db33c canceled.

Name Link
🔨 Latest commit 853e99c
🔍 Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/656f2ce51ac52c0008d5245b

Copy link
Contributor

@bacherfl bacherfl left a comment

Choose a reason for hiding this comment

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

lgtm

@bacherfl
Copy link
Contributor

bacherfl commented Dec 1, 2023

While discussing this, @beeme1mr and I realized it was worth adding a bit more precision here. Please let me know what you folks think, particularly @Kavindu-Dodan and @bacherfl who helped implement some of these.

The main reason this is important, is I will be creating some "negative" gherkin tests to make sure bad inputs and mis-configurations behave consistently across all flagd engine implementations.

Approved, to me it makes sense to be more specific here, and I agree that for boolean results returning false instead of nil is the better option

@toddbaert toddbaert changed the title docs: improve spec precision on bad input fix: improve spec precision on bad input Dec 4, 2023
@toddbaert toddbaert force-pushed the docs/improve-spec-precision branch 2 times, most recently from 1abec3d to d355b77 Compare December 4, 2023 15:16
Signed-off-by: Todd Baert <[email protected]>
Copy link

codecov bot commented Dec 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2293f0e) 72.17% compared to head (853e99c) 71.87%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1041      +/-   ##
==========================================
- Coverage   72.17%   71.87%   -0.30%     
==========================================
  Files          28       28              
  Lines        2864     2866       +2     
==========================================
- Hits         2067     2060       -7     
- Misses        695      701       +6     
- Partials      102      105       +3     

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

Signed-off-by: Todd Baert <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
@toddbaert
Copy link
Member Author

@bacherfl please re-review when you can.

Signed-off-by: Todd Baert <[email protected]>
@toddbaert toddbaert changed the title fix: improve spec precision on bad input fix: various edge cases in targeting Dec 4, 2023
Copy link
Contributor

@bacherfl bacherfl left a comment

Choose a reason for hiding this comment

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

thanks for the update @toddbaert - conceptually that makes perfect sense. just had some minor suggestions regarding the code, but when those are fixed this PR should be good to go :)

@toddbaert toddbaert merged commit ca38c16 into main Dec 5, 2023
15 of 16 checks passed
@github-actions github-actions bot mentioned this pull request Dec 5, 2023
@toddbaert toddbaert deleted the docs/improve-spec-precision branch December 5, 2023 15:43
toddbaert pushed a commit that referenced this pull request Dec 5, 2023
🤖 I have created a release *beep* *boop*
---


<details><summary>flagd: 0.7.2</summary>

##
[0.7.2](flagd/v0.7.1...flagd/v0.7.2)
(2023-12-05)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.7.1
([#1037](#1037))
([0ed9b68](0ed9b68))
</details>

<details><summary>flagd-proxy: 0.3.2</summary>

##
[0.3.2](flagd-proxy/v0.3.1...flagd-proxy/v0.3.2)
(2023-12-05)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.7.1
([#1037](#1037))
([0ed9b68](0ed9b68))
</details>

<details><summary>core: 0.7.2</summary>

##
[0.7.2](core/v0.7.1...core/v0.7.2)
(2023-12-05)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/open-feature-operator
to v0.5.0 ([#1039](#1039))
([eb128d9](eb128d9))
* **deps:** update module github.com/open-feature/open-feature-operator
to v0.5.1 ([#1046](#1046))
([0321935](0321935))
* **deps:** update module golang.org/x/crypto to v0.16.0
([#1033](#1033))
([b79aaf2](b79aaf2))
* **deps:** update module golang.org/x/net to v0.19.0
([#1034](#1034))
([c6426b2](c6426b2))
* various edge cases in targeting
([#1041](#1041))
([ca38c16](ca38c16))
</details>

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

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

5 participants