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.

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

Improve portability, explicitly use GNU/make on macOS #7

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yankcrime
Copy link
Member

No description provided.

@yankcrime yankcrime requested a review from spjmurray June 18, 2024 12:52
@@ -8,4 +8,10 @@
CHART="$(ls charts)"
CHART_META="charts/${CHART}/Chart.yaml"

make images-kind-load VERSION="$(yq .version "${CHART_META}")"
if [[ "$(uname)" == "Darwin" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

I mean, given

[Wed 19 Jun 11:34:08 BST 2024] simon@symphony ~/src/github.com/unikorn-cloud/scripts which gmake
/usr/bin/gmake

I wouldn't be opposed to just changing the command rather than having a sprawling mess of control flow

Copy link
Member Author

Choose a reason for hiding this comment

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

Well now that you mention it, what I actually want to do is to state that developers on macOS should use Colima, and then this build command becomes gmake build images.

Copy link
Member

Choose a reason for hiding this comment

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

Well then, given kubeclt version doesn't give away the platform, and a special flag will be intrusive to someone, I'd consider switching the command based on a config file, say ~/.config/unikorn/scipts.yaml. That would look like...

platform: kind

Then do

MAKE_TARGET=images

if [[ "$(yq .platform /.config/unikorn/scipts.yaml)" == "kind" ]]; then
  MAKE_TARGET=images-kind-load
fi

gmake "${MAKE_TARGET}" ...blah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants