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

Add initial README docs. #47

Merged
merged 1 commit into from
Feb 1, 2018
Merged

Conversation

bobcatfish
Copy link
Contributor

These docs are focused on how to start developing for Elafros.

We should probably aim to simplify this to just a couple of steps,
e.g. the k8s docs
(https://github.com/kubernetes/kubernetes#to-start-developing-kubernetes)
seem to make setup look like just a couple of commands (of course
they link to a lot of docs after the initial Go and Docker
configuration).

I've tried to make these docs very beginner friendly, which I think
is a good foot to start off on.

Fixes #28

@bobcatfish bobcatfish requested review from dewitt and mchmarny January 31, 2018 22:56
sample/README.md Outdated
1. [Setup your development environment](../DEVELOPMENT.md#getting-started)
2. [Start Elafros](../README.md#start-elafros)

## Running samples
Copy link
Member

Choose a reason for hiding this comment

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

I think we should have sample/README.md and a README.md per sample too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kk, ill put the helloworld docs into their own README.md and link from here

@mchmarny
Copy link
Member

mchmarny commented Jan 31, 2018

LGTM. Given the different spots people can start from, would it make sense to split this into multiple docs? Probably longer term goal (Wiki?)

DEVELOPMENT.md Outdated

```shell
$ mkdir -p ${GOPATH}/src/elafros
$ git clone git@github.com:${YOUR_GITHUB_USERNAME}/elafros.git
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to say something here like;
git remote add upstream git@github.com:google/elafros.git
git remote set-url --push upstream no_push

So that git remote -v looks something like this:
vaikas@vaikas-linux3:~/projects/go/src/github.com/google/elafros$ git remote -v
origin git@github.com:vaikas-google/elafros.git (fetch)
origin git@github.com:vaikas-google/elafros.git (push)
upstream git@github.com:google/elafros.git (fetch)
upstream no_push (push)

I've found this super easy to keep fork up to date.
But others might know of a better way.
@mattmoor @grantr

Copy link
Member

Choose a reason for hiding this comment

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

I did:

go get github.com/google/elafros
cd ${GOPATH}/src/github.com/google/elafros
git remote rename origin upstream
git remote add ${USER} github.com/${USER}/elafros

Copy link
Member

Choose a reason for hiding this comment

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

FWIW as-is what you have won't work, you need the github.com/google in the path.

Copy link
Contributor

Choose a reason for hiding this comment

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

seems to work just fine, I can fetch from upstream master, rebase and push to origin master and things are in sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, added!

Copy link
Member

Choose a reason for hiding this comment

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

If you run: ./hack/update-deps.sh then dep will screw up all your deps.

Copy link
Member

Choose a reason for hiding this comment

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

@vaikas-google I mean ${GOPATH}/src/elafros, not what you have, which has the github.com part

DEVELOPMENT.md Outdated
### Iterating

As you make changes to the code-base, there are two special cases to be aware of:
* **If you change a type definition,** then you must run [`./hack/update-codegen.sh`](./hack/update-codegen.sh).
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe explain what a type definition is (not a go type), but pkg/apis/ela/v1alpha1/... files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added! Let me know if I was too specific though and you think there should be more of an explanation.

@bobcatfish
Copy link
Contributor Author

LGTM. Given the different spots people can start from, would it make sense to split this into multiple docs? Probably longer term goal (Wiki?)

@mchmarny I would certainly love to see some docs somewhere re. how to setup a k8s cluster, since that could be a pretty big hurdle to new contributors

Personally I've had better experiences with docs in github rather than a wiki - with markdown (and even RST if you want to get really fancy) you can still have lots of simple docs that work well without a wiki - e.g. the k8s developer docs https://github.com/kubernetes/kubernetes#to-start-developing-kubernetes

@bobcatfish
Copy link
Contributor Author

@mchmarny @vaikas-google @mattmoor PTAL

DEVELOPMENT.md Outdated
repo](https://help.github.com/articles/fork-a-repo/), then:

```shell
mkdir -p ${GOPATH}/src/elafros
Copy link
Member

Choose a reason for hiding this comment

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

This path won't work, it must be ${GOPATH}/src/github.com/google/elafros.

Otherwise ./hack/update-deps.sh will have diffs :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops, thanks, fixed!

1. [Setup your development environment](../DEVELOPMENT.md#getting-started)
2. [Start Elafros](../README.md#start-elafros)

## Running
Copy link
Member

Choose a reason for hiding this comment

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

Include a bit on cleaning up the demo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay done! i also changed the demo from googlecustomer to myhost

Once the codegen and dependency information is correct, redeploying the controller is simply:
```shell
bazel run :controller.replace
```
Copy link
Member

Choose a reason for hiding this comment

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

Should this talk about cleanup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a section in the main README about cleanup and linked to it here, let me know if that's not what you had in mind.

$ kubectl -n ela-system logs $(kubectl -n ela-system get pods -l app=ela-controller -o name)
```

## Clean up
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mattmoor I changed this section to work around bazelbuild/rules_k8s#97 but let me know if you'd rather I changed it back (or if it could be improved)

Copy link
Member

Choose a reason for hiding this comment

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

Does just running :elafros.delete work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well it fails on the webhook delete, cuz of the ordering issue:

➜  elafros git:(28_initial_readme) bazel run :elafros.delete
INFO: Analysed target //:elafros.delete (1 packages loaded).
INFO: Found 1 target...
Target //:elafros.delete up-to-date:
  bazel-bin/elafros.delete
INFO: Elapsed time: 1.111s, Critical Path: 0.01s
INFO: Build completed successfully, 4 total actions

INFO: Running command line: bazel-bin/elafros.delete
namespace "ela-system" deleted
serviceaccount "ela-controller" deleted
clusterrolebinding "ela-controller-admin" deleted
customresourcedefinition "elaservices.elafros.dev" deleted
customresourcedefinition "revisiontemplates.elafros.dev" deleted
customresourcedefinition "revisions.elafros.dev" deleted
deployment "ela-controller" deleted
Error from server (NotFound): error when stopping "/usr/local/google/home/christiewilson/.cache/bazel/_bazel_christiewilson/585c8b345c232690c415b8f36dd1c661/execroot/elafros/bazel-out/k8-fastbuild/bin/elafros.delete.runfiles/elafros/webhook.yaml": deployments.extensions "ela-webhook" not found
ERROR: Non-zero return code '1' from command: Process exited with status 1

so the options are:

  1. run a simpler command that fails with a short error
  2. run several commands that will not fail ?

i think you're leaning toward option 1, so we can go with that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay section is updated @mattmoor

README.md Outdated
bazel run :authz.delete
bazel run :namespace.delete

kubectl delete namespace istio-system
Copy link
Member

Choose a reason for hiding this comment

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

this seems like the wrong order?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, also the istio delete does the namespace delete first thing so this is not needed

$ kubectl -n ela-system logs $(kubectl -n ela-system get pods -l app=ela-controller -o name)
```

## Clean up
Copy link
Member

Choose a reason for hiding this comment

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

Does just running :elafros.delete work?

@bobcatfish bobcatfish force-pushed the 28_initial_readme branch 4 times, most recently from 8f056e7 to bfabd89 Compare February 1, 2018 00:14
These docs are focused on how to start developing for Elafros.

We should probably aim to simplify this to just a couple of steps,
e.g. the k8s docs
(https://github.com/kubernetes/kubernetes#to-start-developing-kubernetes)
seem to make setup look like just a couple of commands (of course
they link to a lot of docs after the initial Go and Docker
configuration).

I've tried to make these docs very beginner friendly, which I think
is a good foot to start off on.

Fixes #28
@bobcatfish bobcatfish merged commit 790dec6 into knative:master Feb 1, 2018
@bobcatfish bobcatfish deleted the 28_initial_readme branch February 1, 2018 01:01
mgencur pushed a commit to mgencur/serving-1 that referenced this pull request Apr 5, 2019
OLM manifests, including CatalogSource
matzew pushed a commit to matzew/serving that referenced this pull request May 2, 2019
OLM manifests, including CatalogSource
tcnghia pushed a commit to tcnghia/serving that referenced this pull request Jun 8, 2020
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.

5 participants