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

topic(build): adding .go-version to track latest go version supported #8181

Merged
merged 2 commits into from Sep 6, 2022

Conversation

darkn3rd
Copy link
Contributor

Problem

Dgraph is built in different go versions that are inconsistent, such as go1.14 or go.1.16. Some versions can cause non-deterministic results when troubleshooting issues, and this could violate any compliance industry should locking down versions of libraries becomes important.

Solution

This adds versioning mechanism to lock latest go version supported by Dgraph. Scripts can be made to use this as a source of truth to explicitly set the version of go.

@joshua-goldstein
Copy link
Contributor

Is the go prefix necessary? The Go images here are tagged with just the numbers. This previously caused an issue with a test script in the Ristretto repo (see this PR).

.go-version Outdated
@@ -0,0 +1 @@
go1.18.5
Copy link
Contributor

Choose a reason for hiding this comment

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

@joshua-goldstein there are 2 issues here:

  • test issue which you raised: this can be addressed by modifying the contents of the .go-version file from go1.18.5 -> 1.18.5 (IMO this would also make installing particular go-version more easier without having to do some regex to remove the go etc.)
  • standardize go version for builds: I think this is needed long-term to pin to a particular environment of go, this would keep the core-code / docker environments / test code etc. all tied to .go-version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes exactly, I agree completely with the second point. 👍 Just wanted to point out the first point you mentioned.

@skrdgraph
Copy link
Contributor

@joshua-goldstein - I have made the fixes in 6387160, I think it's safe to merge.

FYI @darkn3rd ...

could you review?

@skrdgraph skrdgraph merged commit dcc5df1 into main Sep 6, 2022
@skrdgraph skrdgraph deleted the joaquin/go-version branch September 6, 2022 18:11
dshekhar95 pushed a commit that referenced this pull request Sep 12, 2022
…#8181)

* Adding .go-version to track latest go version supported

* fixing the version to just 1.18 vs go1.18

Co-authored-by: Sudhish <108091997+skrdgraph@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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants