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

docs: add a simple local development guide #28

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

joekr
Copy link
Member

@joekr joekr commented Mar 9, 2022

What this PR does / why we need it:
Simply adds a markdown doc for local development. I keep forgetting what the auth config and registry ENVs when doing development work

Which issue(s) this PR fixes:
No open issues for this PR

@joekr joekr added the documentation Improvements or additions to documentation label Mar 9, 2022
@joekr joekr self-assigned this Mar 9, 2022
DEVELOPMENT.md Outdated Show resolved Hide resolved
DEVELOPMENT.md Outdated Show resolved Hide resolved
DEVELOPMENT.md Outdated Show resolved Hide resolved
DEVELOPMENT.md Outdated Show resolved Hide resolved
DEVELOPMENT.md Outdated Show resolved Hide resolved
Copy link
Member

@hyder hyder left a comment

Choose a reason for hiding this comment

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

Please add more explicit steps.

@joekr joekr force-pushed the add-development-doc branch from 76c54c5 to 8a082c4 Compare March 10, 2022 14:35
@joekr joekr requested a review from hyder March 10, 2022 19:29
@Djelibeybi
Copy link
Member

Can you please lint all your Markdown files using markdownlint, please.

@Djelibeybi
Copy link
Member

I suggest following the same layout as the AWS development guide: https://cluster-api-aws.sigs.k8s.io/development/development.html, i.e. start with explaining the prequisites, then set up the environment, then how to commit, etc.

@joekr joekr force-pushed the add-development-doc branch from 8a082c4 to eed3e59 Compare March 11, 2022 19:18
@joekr
Copy link
Member Author

joekr commented Mar 11, 2022

@Djelibeybi ended up moving the doc into the book like you pointed out and expanding upon the developer guide a bit.

@joekr joekr force-pushed the add-development-doc branch from eed3e59 to ff36a79 Compare March 11, 2022 19:50
docs/src/SUMMARY.md Outdated Show resolved Hide resolved
@joekr joekr force-pushed the add-development-doc branch from ff36a79 to 73bcbb1 Compare March 14, 2022 00:00
@joekr joekr requested a review from hyder March 14, 2022 00:00
Copy link
Member

@hyder hyder left a comment

Choose a reason for hiding this comment

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

feedback provided

@joekr joekr force-pushed the add-development-doc branch from 73bcbb1 to d8198f2 Compare March 14, 2022 14:37
@joekr joekr requested a review from hyder March 14, 2022 14:37
Copy link
Member

@Djelibeybi Djelibeybi left a comment

Choose a reason for hiding this comment

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

This is really good, but it's missing the process to lint, test and then submit changes back upstream, which is possibly because we haven't defined that yet. If we have (or if the SIG has preferred options for each), it would be great if those could be documented.

docs/src/development/development.md Outdated Show resolved Hide resolved
docs/src/development/development.md Outdated Show resolved Hide resolved
docs/src/development/development.md Outdated Show resolved Hide resolved
docs/src/development/development.md Outdated Show resolved Hide resolved
docs/src/development/development.md Outdated Show resolved Hide resolved
docs/src/development/development.md Outdated Show resolved Hide resolved
docs/src/development/development.md Outdated Show resolved Hide resolved
hack/auth-config-template.yaml Outdated Show resolved Hide resolved
hack/auth-config-template.yaml Outdated Show resolved Hide resolved
hack/auth-config-template.yaml Show resolved Hide resolved
Copy link
Member

@Djelibeybi Djelibeybi left a comment

Choose a reason for hiding this comment

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

Whoops, clicked the wrong button.

@joekr joekr force-pushed the add-development-doc branch from d8198f2 to a2eaea1 Compare March 16, 2022 20:38
@joekr joekr requested a review from Djelibeybi March 16, 2022 20:38
docs/src/development/development.md Outdated Show resolved Hide resolved
@joekr joekr force-pushed the add-development-doc branch from a2eaea1 to 05dabe7 Compare March 17, 2022 13:19
@joekr joekr requested a review from Djelibeybi March 17, 2022 13:21
Comment on lines +5 to +9
1. Install make.
- `$ xcode-select --install` on macOS.
- `$ sudo apt-get install build-essential` on Ubuntu Linux
- `$ sudo dnf install make gcc` on Oracle Linux
1. Install [Go][go]
Copy link
Member

Choose a reason for hiding this comment

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

Try this:

Suggested change
1. Install make.
- `$ xcode-select --install` on macOS.
- `$ sudo apt-get install build-essential` on Ubuntu Linux
- `$ sudo dnf install make gcc` on Oracle Linux
1. Install [Go][go]
1. Install the required build tools for each operating system:
- macOS: `xcode-select --install`
- Oracle Linux: `sudo yum install make gcc`
- Ubuntu Linux: `sudo apt-get install build-essential`
1. Install [Go][go] by following the [installation guide](https://go.dev/doc/install).

Copy link
Member

Choose a reason for hiding this comment

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

The OSs are sorted alphabetically too.

Comment on lines +10 to +11
1. Install [KIND][kind]
- `$ GO111MODULE="on" go get sigs.k8s.io/[email protected]`.
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
1. Install [KIND][kind]
- `$ GO111MODULE="on" go get sigs.k8s.io/[email protected]`.
1. Install [KIND][kind] by running the following command on all platforms: `$ GO111MODULE="on" go get sigs.k8s.io/[email protected]`.

Comment on lines +12 to +13
1. Install [Kustomize][kustomize]
- [install instructions][kustomizelinux]
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
1. Install [Kustomize][kustomize]
- [install instructions][kustomizelinux]
1. Install [Kustomize][kustomize] by following the [install instructions][kustomizelinux].

Comment on lines +14 to +15
1. Install [envsubst][envsubst]
- `$ go get github.com/a8m/envsubst/cmd/envsubst`
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
1. Install [envsubst][envsubst]
- `$ go get github.com/a8m/envsubst/cmd/envsubst`
1. Install [envsubst][envsubst] by running the following command on all platforms: `go get github.com/a8m/envsubst/cmd/envsubst`

1. Install [envsubst][envsubst]
- `$ go get github.com/a8m/envsubst/cmd/envsubst`

## Fork and get the source
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
## Fork and get the source
## Fork and clone this repository

Comment on lines +38 to +40
If you have `capoci-controller-manager` running in your management cluster,
please scale down the deployment, otherwise running your development build will conflict with the
currently running `capoci-controller-manager`:
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
If you have `capoci-controller-manager` running in your management cluster,
please scale down the deployment, otherwise running your development build will conflict with the
currently running `capoci-controller-manager`:
> **Note:** If you are re-using an existing management cluster that already has `capoci-controller-manager` installed, be sure to provide sufficient resources for both instances or scale down the existing instances to release resources for the new instance.

Comment on lines +46 to +48
To build, run and test all your code changes locally, copy the
`hack/auth-config-template.yaml` file to `<path-to-your-repo>/auth-config.yml` in your
cloned copy of the repository and modify it to match your local configuration.
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
To build, run and test all your code changes locally, copy the
`hack/auth-config-template.yaml` file to `<path-to-your-repo>/auth-config.yml` in your
cloned copy of the repository and modify it to match your local configuration.
To build, run and test all your code changes locally, copy the `hack/auth-config-template.yaml` to `hack/auth-config.yml` and modify it to match your local configuration.

`hack/auth-config-template.yaml` file to `<path-to-your-repo>/auth-config.yml` in your
cloned copy of the repository and modify it to match your local configuration.

Then run the following commands:
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
Then run the following commands:
After populating the local configuration file to suit your envornment, the following command will run the test suite:

Then run the following commands:

```bash
export AUTH_CONFIG_DIR="<path-to-your-repo>/auth-config.yaml"
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
export AUTH_CONFIG_DIR="<path-to-your-repo>/auth-config.yaml"
export AUTH_CONFIG_DIR="hack/auth-config.yaml"

following steps:

```bash
export TAG=<tag>
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
export TAG=<tag>
export TAG="$(git rev-parse --short HEAD)"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants