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.

Sign up for GitHub

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

Jump to bottom

feat: Add go version check #2095

Merged
merged 3 commits into from
Jan 30, 2023
Merged

feat: Add go version check #2095

merged 3 commits into from
Jan 30, 2023

Conversation

glnro
Copy link
Contributor

@glnro glnro commented Jan 29, 2023

This PR addresses concerns raised in #2016 by adding a check in the Makefile to ensure the system is using 1.18 to build Gaia.

AC

$ go version
go version go1.19.5 darwin/arm64

$ make build
ERROR: Go version 1.18 is required for glnro/version-warning-983a9fe4288d6e474aea7402afc640fcbc67e842 of Gaia.
exit 1
make: *** [check_version] Error 1

// Downgrade to 1.18

$ go version
go version go1.18.10 darwin/arm64

$ make build
--> Ensure dependencies have not been modified
all modules verified
...

mpoke
mpoke approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@mpoke mpoke left a comment

Choose a reason for hiding this comment

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

Could we pull the required go version from the SDK version that we are using? :)

MSalopek
MSalopek reviewed Jan 30, 2023
View reviewed changes
Makefile Outdated
###############################################################################

check_version:
ifneq ($(GO_MINOR_VERSION),18)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would there be a problem with checking against 1.18 or even >= 1.18?

Also, would it be possible to make the hardcoded 18 const into a var?

REQUIRE_GO_VERSION = '1.18'

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@glnro glnro merged commit c9650f8 into main Jan 30, 2023
@glnro glnro deleted the glnro/version-warning branch January 30, 2023 15:21
mergify bot pushed a commit that referenced this pull request Jan 30, 2023
@glnro @mergify
feat: Add go version check (#2095)
5ad9b40 
* Add go version check

* Make requested changes

(cherry picked from commit c9650f8)
@mergify mergify bot mentioned this pull request Jan 30, 2023
Merged
mergify bot pushed a commit that referenced this pull request Jan 30, 2023
@glnro @mergify
feat: Add go version check (#2095)
d798d29 
* Add go version check

* Make requested changes

(cherry picked from commit c9650f8)
@mergify mergify bot mentioned this pull request Jan 30, 2023
Merged
glnro added a commit that referenced this pull request Jan 30, 2023
@mergify @glnro
feat: Add go version check (#2095) (#2103)
fc32da4 
* Add go version check

* Make requested changes

(cherry picked from commit c9650f8)

Co-authored-by: lg <8335464+glnro@users.noreply.github.com>
glnro added a commit that referenced this pull request Jan 30, 2023
@mergify @glnro
feat: Add go version check (#2095) (#2102)
1e97fc2 
* Add go version check

* Make requested changes

(cherry picked from commit c9650f8)

Co-authored-by: lg <8335464+glnro@users.noreply.github.com>
@zivkovicmilos zivkovicmilos mentioned this pull request Aug 29, 2024
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MSalopek MSalopek MSalopek approved these changes

@mpoke mpoke mpoke approved these changes

@alexanderbez alexanderbez Awaiting requested review from alexanderbez

@zmanian zmanian Awaiting requested review from zmanian

@yaruwangway yaruwangway Awaiting requested review from yaruwangway

@crodriguezvega crodriguezvega Awaiting requested review from crodriguezvega

@jackzampolin jackzampolin Awaiting requested review from jackzampolin

@mmulji-ic mmulji-ic Awaiting requested review from mmulji-ic

@jtremback jtremback Awaiting requested review from jtremback

@sainoe sainoe Awaiting requested review from sainoe

@shaspitz shaspitz Awaiting requested review from shaspitz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@glnro @mpoke @MSalopek