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
Add new makefile targets for go mod verification #3550
base: master
Are you sure you want to change the base?
Conversation
Run 'go mod' functions to search for tidy, vendor changes and verify it
Update makefile and one doc file docomentation with the change of go-verify over using vendor and tidy
/azp run ci |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -167,7 +170,7 @@ run-portal: | |||
go run -ldflags "-X github.com/Azure/ARO-RP/pkg/util/version.GitCommit=$(VERSION)" ./cmd/aro portal | |||
|
|||
build-portal: | |||
cd portal/v2 && npm install && npm run build | |||
cd portal/v1 && npm install && npm run build && cd ../v2 && npm install && npm run build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this line? It was deleted as part of PR #3465 that removed portal/v1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good to me, except what @tiguelu already pointed about. Once thats changed. LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice change. Just a few questions
@@ -266,9 +269,22 @@ admin.kubeconfig: | |||
aks.kubeconfig: | |||
hack/get-admin-aks-kubeconfig.sh | |||
|
|||
.PHONY: go-tidy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why each individual PHONY and not added to the overall PHONY at the bottom of the file?
@@ -18,6 +18,9 @@ GATEKEEPER_VERSION = v3.10.0 | |||
GATEKEEPER_IMAGE ?= ${RP_IMAGE_ACR}.azurecr.io/gatekeeper:$(GATEKEEPER_VERSION) | |||
GOTESTSUM = gotest.tools/gotestsum@v1.11.0 | |||
|
|||
# Golang version go mod tidy compatibility | |||
GOLANG_VERSION ?= 1.20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we specify 1.20.12? (Still finding my feet with Go, I have no idea how strict it is with versions)
For reference: #3548
Which issue this PR addresses:
None. It is a small enhancement. Duplicate of #3492.
What this PR does / why we need it:
Run 'go mod' functions to search for tidy, and vendor package changes and then verify it.
Test plan for issue:
Is there any documentation that needs to be updated for this PR?
Yes, in the second commit
How do you know this will function as expected in production?