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

refactor: migrate to connectrpc/connect-go #372

Merged
merged 6 commits into from
Nov 14, 2023
Merged

refactor: migrate to connectrpc/connect-go #372

merged 6 commits into from
Nov 14, 2023

Conversation

craigpastro
Copy link
Member

This PR

Migrate from bufbuild/connect-go to connectrpc/connect-go.

Part of open-feature/flagd#790. Related to open-feature/flagd#990.

There is some trouble with circular-ish dependencies though. This needs to be merged for the integration test in open-feature/flagd#990 to pass, but this needs open-feature/flagd#990 to be merged to compile. I tested locally with a replace statement and everything was fine.

This PR also cleans up imports and moves to go.uber.org/mock from github.com/golang/mock which is no longer maintained.

@Kavindu-Dodan
Copy link
Contributor

I released upstream flagd. However, we need to perform two more steps before acepting this

  1. Fix releases propsed through this PR - fix: fix flagd dependencies #380
  2. Release flagd testing and adopt latest flagd core release and testing release

Then we will not have the test failure caused by multiple protobuf registrations

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.

I have to block this till #372 (comment) is done. Otherwise thank you for taking care of this @craigpastro 🤝

craigpastro and others added 6 commits November 14, 2023 09:35
Signed-off-by: Craig Pastro <craig.pastro@gmail.com>
Signed-off-by: Craig Pastro <craig.pastro@gmail.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
Copy link
Member

toddbaert commented Nov 14, 2023

@Kavindu-Dodan I did what you described here and tests are now passing. (I also had to update the go version in the CI to 1.20 (1.19 is deprecated now) to fix a compilation issue. I think we may want to update all go.mods to use v1.20, but lets do that in a separate PR.

@Kavindu-Dodan
Copy link
Contributor

@Kavindu-Dodan I did what you described here and tests are now passing. (I also had to update the go version in the CI to 1.20 (1.19 is deprecated now) to fix a compilation issue. I think we may want to update all go.mods to use v1.20, but lets do that in a separate PR.

Awesome, thanks for taking care of the dependency bump 🤝

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 🎉

@Kavindu-Dodan Kavindu-Dodan merged commit aba4f4e into open-feature:main Nov 14, 2023
5 checks passed
@Kavindu-Dodan
Copy link
Contributor

@craigpastro thank you again for the contribution :)

@craigpastro
Copy link
Member Author

My pleasure!

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.

None yet

4 participants