Skip to content

feat:add embedded-resources target in Makefile#233

Merged
nabuskey merged 2 commits intocnoe-io:mainfrom
iron87:feature-make-target
May 1, 2024
Merged

feat:add embedded-resources target in Makefile#233
nabuskey merged 2 commits intocnoe-io:mainfrom
iron87:feature-make-target

Conversation

@iron87
Copy link
Copy Markdown
Contributor

@iron87 iron87 commented Apr 29, 2024

resolve #136

resolve cnoe-io#136

Signed-off-by: iron87 <fede.cerruto@gmail.com>
Copy link
Copy Markdown
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

Signed-off-by: iron87 <fede.cerruto@gmail.com>
Copy link
Copy Markdown
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
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