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

feat:add embedded-resources target in Makefile #233

Merged
merged 2 commits into from
May 1, 2024

Conversation

iron87
Copy link
Contributor

@iron87 iron87 commented Apr 29, 2024

resolve #136

Copy link
Collaborator

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

Thank you very much for this PR!

I don't think I was specific enough in the issue. That's my bad. Instead of embedding bash scripts in the Makefile, let's create a script in the hack directory so that it's easier to edit in the future. In the script we should specify directory names since we know which directories we care about. Something like this:

hack/embedded-resources.sh

#!/bin/bash

for dir in argo-cd gitea ingress-nginx; do
    ./hack/$dir/generate-manifests.sh;
    if [[ $? -ne 0 ]]; then
        echo "error running script: ./hack/$dir/generate-manifests.sh"
        exit 1
    fi
done

Then we can run it like so in Makefile:

.PHONY: embedded-resources
embedded-resources:
	./hack/embedded-resources.sh

Copy link
Collaborator

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you very much for this PR!

@nabuskey nabuskey merged commit 6a95dce into cnoe-io:main May 1, 2024
2 checks passed
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.

Make target for generating embedded resources
2 participants