Skip to content

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
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.
4 participants