Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert requested a review from a team as a code owner November 30, 2023 14:42
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 <todd.baert@dynatrace.com>
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 <todd.baert@dynatrace.com>
@toddbaert toddbaert requested a review from bacherfl December 4, 2023 15:29
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
Copy link
Member Author

@bacherfl please re-review when you can.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert changed the title fix: improve spec precision on bad input fix: various edge cases in targeting Dec 4, 2023
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
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 and others added 2 commits December 5, 2023 08:51
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert merged commit ca38c16 into main Dec 5, 2023
@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>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants