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

doc: make implicity the users must set envtest #3709

Closed

Conversation

camilamacedo86
Copy link
Contributor

Description of the change:
Update the title to make clear to the users that the must configure the envtest

Motivation for the change:
The issue raised: #3692

@camilamacedo86
Copy link
Contributor Author

Hi @jmrodri and @jmccormick2001,

WDYT about this change in the doc until we are able to solve it when v3+ plugin be supported?

@@ -34,7 +34,7 @@ Create a simple Memcached API:
operator-sdk create api --group cache --version v1 --kind Memcached --resource=true --controller=true
```

### Configuring your test environment
### You must configure your test environment
Copy link
Contributor

Choose a reason for hiding this comment

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

Configure Test Environment (REQUIRED for building the operator)

Copy link
Member

Choose a reason for hiding this comment

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

I disagree with changing the headers. Can we add a sentence in this section (and possibly in the section that includes make docker-build) about the reasons for the necessity of this step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let us know wdyt now @joelanford ?

@jmccormick2001
Copy link
Contributor

one thing you might consider, instead of having a 'setup envtest' section, you might instead just reference how to install the dependencies required to do the docker-build, I found that yesterday in testing some things, you can just follow https://book-v1.book.kubebuilder.io/getting_started/installation_and_setup.html to install the kubebuilder dependency, then proceed.

@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Aug 18, 2020

Hi @jmccormick2001

one thing you might consider, instead of having a 'setup envtest' section, you might instead just reference how to install the dependencies required to do the docker-build, I found that yesterday in testing some things, you can just follow https://book-v1.book.kubebuilder.io/getting_started/installation_and_setup.html to install the kubebuilder dependency, then proceed.

It is true, teh setup is not required If users install the kubebuilder. However, kubebuilder should NOT be a pre-requirement for users are able to use SDK. The setup in the quick-start will NOT install kubebuilder just the required binaries in the project instead of also requires sudo permission as described in the Kubebuilder installation doc.

With the implementation in the PR kubernetes-sigs/kubebuilder#1626 we are doing this update in the Makefile that is generated by the GO plugin which also is required for we achieve other requirements for Kubebuilder as well. See that in the long term we should no longer provide these binaries with kubebuilder kubernetes-sigs/kubebuilder#686. Anyway, the step in the doc will be required for a while since the change is valid just for v3+ plugin and we are using in SDK the v2 plugin version.

I hope that clarifies.

@camilamacedo86 camilamacedo86 added the kind/documentation Categorizes issue or PR as related to documentation. label Aug 30, 2020
@camilamacedo86
Copy link
Contributor Author

Close this one in favor of: #3983

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Categorizes issue or PR as related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants