add details to CONTRIBUTING.md #420
Conversation
explain more about what each step does and fix order of dev-requirements which must be installed before running install-hub closes jupyterhub#344
CONTRIBUTING.md
Outdated
| @@ -71,18 +86,6 @@ To run unit tests, navigate to the root of the repository, then call: | |||
|
|
|||
| We recommend increasing your GitHub API rate limit before running tests (see above). | |||
|
|
|||
| ## Building JS and CSS | |||
There was a problem hiding this comment.
Do we not need this any more? Maybe we should leave a pointer here for how to rebuild just the JS and CSS for people exclusively working on the frontend.
CONTRIBUTING.md
Outdated
| This command builds the image using the same docker host as the minikube VM, so that the images are automatically present. | ||
| Note: when you no longer wish to use the minikueb host, you can undo this change by running: | ||
|
|
||
| eval $(minikube docker-env -u) |
There was a problem hiding this comment.
Nice! I did not know this existed :)
CONTRIBUTING.md
Outdated
| eval $(minikube docker-env) | ||
|
|
||
| This command builds the image using the same docker host as the minikube VM, so that the images are automatically present. | ||
| Note: when you no longer wish to use the minikueb host, you can undo this change by running: |
CONTRIBUTING.md
Outdated
| eval $(minikube docker-env) | ||
| eval $(minikube docker-env) | ||
|
|
||
| This command builds the image using the same docker host as the minikube VM, so that the images are automatically present. |
There was a problem hiding this comment.
How about "This command sets up docker to use the same docker daemon as your minikube cluster does. This means images you build are directly available to the cluster"?
CONTRIBUTING.md
Outdated
| @@ -22,7 +24,7 @@ on GitHub if you don't have a token. | |||
| [Alternative methods](https://docs.helm.sh/using_helm/#installing-the-helm-client) for helm installation | |||
| exist if you prefer installing without using the script. | |||
|
|
|||
| 3. Initialize helm in minikube | |||
| 3. Initialize helm in minikube. This command initialize the local CLI and also install Tiller into your Kubernetes in one step: | |||
There was a problem hiding this comment.
initialize -> initializeS, "CLI and also installS", into -> on (??), Kubernetes -> Kubernetes cluster
CONTRIBUTING.md
Outdated
|
|
||
| 2. Install helm | ||
| 2. Install helm to manage Kubernetes charts, |
There was a problem hiding this comment.
I always thought the charts are called "Helm charts"? How about: "Install helm to manage installing and running binderhub on your cluster"?
CONTRIBUTING.md
Outdated
| For MacOS, you may find installing from https://github.com/kubernetes/minikube/releases may be | ||
| more stable than using Homebrew. | ||
|
|
||
| To interact with your minikube cluster, run the command: `minikube start`, this starts a local kubernetes cluster using VM. This command assumes that you have already installed one of the VM drivers: virtualbox/ |
There was a problem hiding this comment.
The end of the sentence got lost I think.
I would replace "To interact with your ..." with "To start your minikube cluster ..." for me this makes it clearer that this isn't an optional thing. You have to start the cluster before going to the next step!!111 :)
|
I added some comments with ideas from my side. Thanks for taking the time to not only work out how things actually work (in spite of our docs) but to then also write it down! 🎉 |
|
+1 to @betatim's points. I think it's good to go once you take another pass for grammar/spelling/type-os etc. Thanks very much @GladysNalvarte ! |
There was a problem hiding this comment.
Nicely done @GladysNalvarte. One small suggestion for clarity. I'm happy to merge with or without the suggested change.
CONTRIBUTING.md
Outdated
| python3 -m pip install -e . -r dev-requirements.txt | ||
| python3 -m pip install -e . -r dev-requirements.txt | ||
|
|
||
| This list of packages is necessary to create an environment that will generate the Docker image using the Git repository. Regardless of what is in the setup.py file, it will install what the user needs to build the Docker image. |
There was a problem hiding this comment.
The list of packages in dev-requirements.txt will be used to create a suitable environment for generating the Docker image for the Git Repository. Regardless ... it the requirements file will ...
|
I went ahead and made @willingc 's patch to the branch. I think this is good to go! Thanks so much @GladysNalvarte - this kind of practical documentation is really helpful in building the community! |
|
🎉 ! (travis failed because you didn't wait for it or because it actually failed?) |
|
it was because of an eager merge on my part - travis passed and there was a one-word edit that I added to the doc before merging, my bad for not updating the final comment with that information |
explain more about what each step does
and fix order of dev-requirements which must be installed before running install-hub
closes #344