Skip to content

chore: bump go to 1.21 #1208

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

Closed
wants to merge 3 commits into from
Closed

chore: bump go to 1.21 #1208

wants to merge 3 commits into from

Conversation

odubajDT
Copy link
Contributor

@odubajDT odubajDT commented Feb 13, 2024

Let's align with the latest Go release

Fixes #1207

Signed-off-by: odubajDT <[email protected]>
Copy link

netlify bot commented Feb 13, 2024

Deploy Preview for polite-licorice-3db33c canceled.

Name Link
🔨 Latest commit d5caf8c
🔍 Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/65cb6dc252c22c00081ee7e6

Signed-off-by: odubajDT <[email protected]>
@odubajDT odubajDT marked this pull request as ready for review February 13, 2024 13:48
@odubajDT odubajDT requested a review from a team as a code owner February 13, 2024 13:48
@@ -1,6 +1,6 @@
module github.com/open-feature/flagd/core

go 1.20
go 1.21
Copy link
Member

Choose a reason for hiding this comment

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

My only concern here is that the flagd go provider uses this module as a library, and it's using 1.20: https://github.com/open-feature/go-sdk-contrib/blob/main/providers/flagd/go.mod

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be also fine if we merge this as well open-feature/go-sdk-contrib#452. 1.21 should be fully backwards compatible with 1.20

Copy link
Member

Choose a reason for hiding this comment

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

If that's the case, I guess it won't even be necessary to update the contribs right now at all to consume this? (not that it isn't a good idea anyway).

cc @thisthat @Kavindu-Dodan any concerns?

Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan Feb 13, 2024

Choose a reason for hiding this comment

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

I think we should not upgrade all providers in the contrib repository. But we can upgrade flagd provider independently and there are providers already using Go 1.21 (hence the pipeline using the latest Go version).

Similarly, we should upgrade Go-SDK gradually as it's a central piece of OF. I know Go has a backward compatibility promise, but once upgraded and if we use a newer API, then it can impact users who are yet to upgrade their Go version. This could happen for enterprise customers where upgrades are slow and time-consuming compared to Go's 6 month release cycle :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I wasn't aware that some of the users are using parts of the flagd code as a library.

This PR might be then a candidate for 1.0 stable version. At least I would vote for aligning all versions to 1.21.

Copy link
Contributor

Choose a reason for hiding this comment

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

@odubajDT it's actually our flagd go provider [1] that depends on the core.

Personally, I am fine with moving ahead with Go 1.21 for flagd. And then we can bump the go provider of flagd itself. But I would avoid changing other providers for sometime :) I hope this is clearer now?

[1] - https://github.com/open-feature/go-sdk-contrib/blob/main/providers/flagd/go.mod#L15

Copy link
Contributor

Choose a reason for hiding this comment

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

ohh, and please check for API changes if any in the PR. If there's any, it might be good to mark the PR as a breaking change as we loose forward compatibility for users

@Kavindu-Dodan
Copy link
Contributor

Closing as this is outdated

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.

[FEATURE] Upgrade to go version 1.21
3 participants