-
Notifications
You must be signed in to change notification settings - Fork 6
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
Configure linting for scripts/ directory #14
Conversation
Signed-off-by: Jasstkn <[email protected]>
8b25de1
to
fb8805a
Compare
@neelanjan00 hi. I wonder if I need to fix issues that have been found by shellcheck in this PR? |
Hi @Jasstkn, thank you so much for the PR! 🙂 It will surely be great if you can resolve the issues in the shell script(s) as well since the CI is failing i.e. the shell scripts indeed require fixes. After the changes, you can also test your changes by installing/uninstalling |
933b3fb
to
901f3d4
Compare
Signed-off-by: Jasstkn <[email protected]>
901f3d4
to
ebe7bbb
Compare
Hey. Looks like on my mac m1 there are some issue with running this script. I've tried to run in CI but it fails because of missing go environment variables. Do you have any ideas how we can handled it easily? For now I just increased the severity level for which shellcheck should fail. |
The shell script checks for the OS and CPU arch before the actual installation. Currently, only Linux is supported for OS but MacOS is Unix aka. Darwin hence it isn't supported. You can use a Linux VM to run the script for testing. |
Then what are the pre-requisites? Go installed? And could you elaborate a bit how it should be configured? Because I'm getting errors about badly configured GOROOT and etc. |
@neelanjan00 ping |
Hi @Jasstkn, sorry for the late reply. To run the m-agent program, you need to have Go installed. Also, there's a file that you need to create in Also, can you please mention the exact steps to reproduce the error and attach a screenshot of the error here? |
Error is the following:
|
@neelanjan00 ping |
Hi @Jasstkn, sorry for the late reply. I am not sure why you're getting this error, when I run |
As you mentioned - I need to have a linux machine. So, I used lima-vm project to achieve this. The output above is from ubuntu virtual machine that I spined up. |
Can you try to run the command that I used above? |
ran successfully.
The current issue is related to build script. |
Got it. Actually, the build script expects ENVs and CLI params such as the package path, tag, target OS, target Architecture, etc. When configured, it builds the m-agent binary for an array of CPU architectures for Linux OS. A more straightforward way to build the binary for testing on your Linux VM is to directly build the code with Assuming your VM is using an
|
I just wanted to verify that script is working properly after my changes. Maybe you can handle this part? |
@neelanjan00 ping |
Signed-off-by: Jasstkn [email protected]
What this PR does / why we need it:
Add GA workflow to lint scripts
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #13Special notes for your reviewer:
Checklist:
breaking-changes
tagrequires-upgrade
tag