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

ansible/helm: add makefile helper #4660

Merged

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Mar 15, 2021

Signed-off-by: Camila Macedo cmacedo@redhat.com

Description of the change:
add makefile helper for ansible/helm

Motivation for the change:
provide for ansible/helm the same nice features that we have for Go
Closes: #4628

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 15, 2021 12:16 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 15, 2021 12:16 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 15, 2021 12:16 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 15, 2021 12:16 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 15, 2021 12:16 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 15, 2021 12:16 Inactive
@jberkhahn
Copy link
Contributor

seems to work fine. I worry about this just being text in a template somewhere, though - one more thing we have to remember to update if anything changes.
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2021
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2021
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 21:59 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 21:59 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 21:59 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 21:59 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 21:59 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 21:59 Inactive
changelog/fragments/ansible-helm-makefile-help.yaml Outdated Show resolved Hide resolved
Comment on lines 33 to 36
Ansible/Helm projects scaffolded by the tool now provides a Makefile
helper. To address these changes you can copy and paste the Makefile from
its samples which are in the testdata. Check the latest version of the
Helm Makefile [here](https://github.com/operator-framework/operator-sdk/blob/v1.6.0/testdata/helm/memcached-operator/Makefile) and check it for Ansible [here](https://github.com/operator-framework/operator-sdk/blob/v1.6.0/testdata/ansible/memcached-operator/Makefile).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Ansible/Helm projects scaffolded by the tool now provides a Makefile
helper. To address these changes you can copy and paste the Makefile from
its samples which are in the testdata. Check the latest version of the
Helm Makefile [here](https://github.com/operator-framework/operator-sdk/blob/v1.6.0/testdata/helm/memcached-operator/Makefile) and check it for Ansible [here](https://github.com/operator-framework/operator-sdk/blob/v1.6.0/testdata/ansible/memcached-operator/Makefile).
Ansible/Helm projects now provide a Makefile `help` target, similar to a `--help` flag.
You can copy and paste this target from the relevant sample's Makefile
([helm]((https://github.com/operator-framework/operator-sdk/blob/v1.5.0/testdata/helm/memcached-operator/Makefile), [ansible]((https://github.com/operator-framework/operator-sdk/blob/v1.5.0/testdata/ansible/memcached-operator/Makefile)).

# Migration can be defined to automatically add a section to
# the migration guide. This is required for breaking changes.
migration:
header: (helm/v1,ansible/v1) - **(Optional)** Add Makefile helper
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
header: (helm/v1,ansible/v1) - **(Optional)** Add Makefile helper
header: (helm/v1, ansible/v1) Add `help` target to Makefile.

Comment on lines 118 to 120
.PHONY: kustomize ## Download kustomize locally if necessary.
KUSTOMIZE = $(shell pwd)/bin/kustomize
kustomize:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.PHONY: kustomize ## Download kustomize locally if necessary.
KUSTOMIZE = $(shell pwd)/bin/kustomize
kustomize:
.PHONY: kustomize
KUSTOMIZE = $(shell pwd)/bin/kustomize
kustomize: ## Download kustomize locally if necessary.

Comment on lines 134 to 136
.PHONY: ansible-operator ## Download ansible-operator locally if necessary, preferring the $(pwd)/bin path over global if both exist.
ANSIBLE_OPERATOR = $(shell pwd)/bin/ansible-operator
ansible-operator:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.PHONY: ansible-operator ## Download ansible-operator locally if necessary, preferring the $(pwd)/bin path over global if both exist.
ANSIBLE_OPERATOR = $(shell pwd)/bin/ansible-operator
ansible-operator:
.PHONY: ansible-operator
ANSIBLE_OPERATOR = $(shell pwd)/bin/ansible-operator
ansible-operator: ## Download ansible-operator locally if necessary, preferring the $(pwd)/bin path over global if both exist.

@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:05 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:06 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:06 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:06 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:06 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:06 Inactive
Signed-off-by: Camila Macedo <cmacedo@redhat.com>
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:11 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:11 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:11 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:11 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:11 Inactive
@camilamacedo86 camilamacedo86 temporarily deployed to deploy March 16, 2021 22:11 Inactive
Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2021
@camilamacedo86 camilamacedo86 merged commit 0b17e16 into operator-framework:master Mar 16, 2021
@camilamacedo86 camilamacedo86 deleted the align-makefile branch March 16, 2021 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ansible/Helm add help to the Makefile
4 participants