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

chore: add instructions for windows and fix failing unit tests #632

Merged
merged 12 commits into from
May 4, 2023

Conversation

RealAnna
Copy link
Contributor

@RealAnna RealAnna commented Apr 27, 2023

This PR

  • fixes unit tests that failed on windows machines
  • clarifies usage of flagd on a windows machine

Related Issues

#630

Notes

The learning from this pr were:

  1. Always prefer using the go test suite tempdir instead of relying on os library to allocate and delete test folders, this simplifies code and avoids os-specific issues
  2. Make sure to always close file pointers, since leaving dangling ones tends to cause access errors in Windows aka error file already open.
  3. When running via docker one must make sure that the file is in the same filesystem where the docker container runs than the watcher will work. AKA on Windows it is good as long as everything comes from WSL filesystem

Signed-off-by: RealAnna <anna.reale@dynatrace.com>

readme, add instructions for windows

Signed-off-by: RealAnna <anna.reale@dynatrace.com>

tests: fix file tests

Signed-off-by: RealAnna <anna.reale@dynatrace.com>
@codecov
Copy link

codecov bot commented Apr 27, 2023

Codecov Report

Merging #632 (094ec8e) into main (28bac6a) will increase coverage by 0.37%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #632      +/-   ##
==========================================
+ Coverage   72.39%   72.77%   +0.37%     
==========================================
  Files          24       24              
  Lines        2384     2384              
==========================================
+ Hits         1726     1735       +9     
+ Misses        592      583       -9     
  Partials       66       66              

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@RealAnna RealAnna changed the title readme, add instructions for windows chore: add instructions for windows and fix failing unit tests Apr 27, 2023
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
@RealAnna RealAnna marked this pull request as ready for review April 27, 2023 11:18
@RealAnna RealAnna requested a review from a team as a code owner April 27, 2023 11:18
README.md Outdated Show resolved Hide resolved
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

LGTM but I agree with @Kavindu-Dodan 's suggestions.

RealAnna added 3 commits May 2, 2023 09:01
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
Signed-off-by: RealAnna <anna.reale@dynatrace.com>
README.md Outdated Show resolved Hide resolved
Co-authored-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: RealAnna <89971034+RealAnna@users.noreply.github.com>
Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan left a comment

Choose a reason for hiding this comment

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

lgtm

@toddbaert toddbaert self-requested a review May 4, 2023 12:15
@toddbaert toddbaert merged commit 6999d67 into open-feature:main May 4, 2023
@github-actions github-actions bot mentioned this pull request May 4, 2023
toddbaert pushed a commit that referenced this pull request May 4, 2023
🤖 I have created a release *beep* *boop*
---


<details><summary>flagd: 0.5.3</summary>

##
[0.5.3](flagd/v0.5.2...flagd/v0.5.3)
(2023-05-04)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.5.2
([#613](#613))
([218f435](218f435))
* **deps:** update module github.com/spf13/cobra to v1.7.0
([#587](#587))
([12b3477](12b3477))


### ✨ New Features

* Introduce connect traces
([#624](#624))
([28bac6a](28bac6a))
</details>

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

##
[0.2.3](flagd-proxy/v0.2.2...flagd-proxy/v0.2.3)
(2023-05-04)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.5.2
([#613](#613))
([218f435](218f435))
* **deps:** update module github.com/spf13/cobra to v1.7.0
([#587](#587))
([12b3477](12b3477))


### ✨ New Features

* Introduce connect traces
([#624](#624))
([28bac6a](28bac6a))
</details>

<details><summary>core: 0.5.3</summary>

##
[0.5.3](core/v0.5.2...core/v0.5.3)
(2023-05-04)


### 🐛 Bug Fixes

* **deps:** update module github.com/bufbuild/connect-go to v1.6.0
([#585](#585))
([8f2f467](8f2f467))
* **deps:** update module github.com/bufbuild/connect-go to v1.7.0
([#625](#625))
([1b24fc9](1b24fc9))
* **deps:** update module github.com/open-feature/open-feature-operator
to v0.2.34 ([#604](#604))
([3e6a84b](3e6a84b))
* **deps:** update module github.com/prometheus/client_golang to v1.15.0
([#608](#608))
([0597a8f](0597a8f))
* **deps:** update module github.com/rs/cors to v1.9.0
([#609](#609))
([97066c1](97066c1))
* **deps:** update module github.com/rs/xid to v1.5.0
([#614](#614))
([e3dfbc6](e3dfbc6))
* **deps:** update module golang.org/x/crypto to v0.8.0
([#595](#595))
([36016d7](36016d7))


### ✨ New Features

* Introduce connect traces
([#624](#624))
([28bac6a](28bac6a))


### 🧹 Chore

* add instructions for windows and fix failing unit tests
([#632](#632))
([6999d67](6999d67))
</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.

4 participants