-
Notifications
You must be signed in to change notification settings - Fork 163
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
Revert "tools: add install_yq" #1400
base: main
Are you sure you want to change the base?
Conversation
This reverts commit d0d3990.
@@ -291,7 +291,6 @@ TOOLS = govulncheck \ | |||
jq \ | |||
kubectl \ | |||
kustomize \ | |||
yq \ |
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.
Instead of removing it, how about we maintian a different target called ci-tools
and have
tools: ci-tools
YQ is quite useful since we have a lot of yaml files in manifests
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.
I'm all for a new target and ci-tools
is ok - but it doesn't encompass the development side.
could we not just have a deps
target that installs all the depdendencies and centralise all the dependencies/tools between the kepler repo and the local-dev-cluster in one repo...
From a developer experience POV - it would make the onboarding experience a lot smoother IMHO. Right now we rely on the developer to manually run a command to install the tools they need after finding the appropriate reference in the Readme rather than automating this step
To improve developer experience. If YQ isn't installed then TBH as a developer it would be great if the first thing I could do after cloning the kepler repo is run IMHO - the kepler developer onboarding experience could be greatly improved. |
I tested |
Reverts #1393
There seems no need for a PR to add yq.
In CI if we have grafana features, which yq installed at https://github.com/sustainable-computing-io/local-dev-cluster/blob/main/main.sh#L190 this covers case which we build up instance on a new created ec2.
and if we need to handle json, yq and jq been installed via GHA naivety.
https://github.com/actions/runner-images/blob/7bb1d84f7071bfa9c350d7552d9631d9a69bfdb0/images/ubuntu/Ubuntu2404-Readme.md?plain=1#L72-L86
So, what's the reason we add yq install in our code?