Skip to content

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 <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
@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
@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.

4 participants