-
Notifications
You must be signed in to change notification settings - Fork 54
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 arch support for local go build #447
Conversation
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
Quality Gate passedIssues Measures |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #447 +/- ##
==========================================
+ Coverage 80.57% 80.63% +0.05%
==========================================
Files 35 35
Lines 5040 5040
==========================================
+ Hits 4061 4064 +3
+ Misses 774 772 -2
+ Partials 205 204 -1 ☔ View full report in Codecov by Sentry. |
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.
/lgtm tested locally with ubuntu (22.04) & amd64
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: maysunfaisal, thepetk The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -52,6 +52,10 @@ APPLICATION_API_CRD = https://raw.githubusercontent.com/redhat-appstudio/applica | |||
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. | |||
ENVTEST_K8S_VERSION = 1.22 | |||
|
|||
# Get the local platform | |||
OS=$(shell go env GOOS) |
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.
Wouldn'tgo build
be able to automatically retrieve these?
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.
Good question, I tried finding this exact verbage for go build on docs but didnt find any https://go.dev/doc/tutorial/compile-install
I tried various random posts and stackoverflow but didnt really find any info about it! 🤔
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.
Honestly I am down to not merging this because I had the same notion before opening this PR.
Closing this, as I echo what John mentioned in the comment, I dont think we need this. |
What does this PR do?:
Add arch support for go build binary in Makefile. Not needed in Dockerfile, since we are building inside the image.
Which issue(s)/story(ies) does this PR fixes:
PR acceptance criteria:
Unit/Functional tests
Documentation
Client Impact
How to test changes / Special notes to the reviewer: