docs: developer documentation #2275
Conversation
There was a problem hiding this comment.
This documentation is great! Sections are on point, easy to follow, and links to tutorials are helpful for new people.
I would modify the prerequisites because client-side development with Cypress would require only npm.
I also added a comment to the server section to mention user tokens -- you said it might require more extensive changes, but it seems already good to me!
README.md
Outdated
| # Prerequisites | ||
|
|
||
| To develop on this codebase, you will want the following tools installed. | ||
|
|
||
|
|
||
| | | | | ||
| |----------------------------------------------------------------------|---------------------------------------------------------------------| | ||
| | [docker](https://www.docker.com) | For building containers | | ||
| | [helm](https://helm.sh) | For packaging things for K8s | | ||
| | [kubectl](https://kubernetes.io/docs/tasks/tools/) | K8s command-line tool | | ||
| | [nvm](https://github.com/nvm-sh/nvm) | NVM or some similar tool for managing node environments | | ||
| | [telepresence](https://www.telepresence.io/docs/latest/quick-start/) | Tool for redirecting requests to your local development environment | | ||
|
|
||
| You should use your **node version manager** to install the node version specified in the [Dockerfile](https://github.com/SwissDataScienceCenter/renku-ui/blob/master/client/Dockerfile). |
There was a problem hiding this comment.
Most of these tools are not required for developing with Cypress on the client. And I believe helm is not needed in most cases, just for deploying to an own namespace but not for interacting with CI deployments.
Should we rephrase to indicate that npm is always necessary, and docker+kubectl are required for telepresence?
There was a problem hiding this comment.
Good suggestion, I have split up the prerequisites into three sections -- always needed, needed for telepresence, and needed to deploy to K8s.
README.md
Outdated
|
|
||
| # Server | ||
|
|
||
| The server is the [Express-based](https://expressjs.com) back-end for the RenkuLab UI. At first, the front-end would interact directly with back-end services, but this structure imposed some limitations and complications, so we introduced a server component with the sole responsibility of serving the UI and simplifying interactions with the other backend services. |
There was a problem hiding this comment.
I would remove the historical reasons for creating the server, and mention that it also handles requesting, storing, and renewing user tokens because it's safer than doing it in the client and it allows us to poll some resources on behalf of the user
There was a problem hiding this comment.
Good idea. I have made these changes.
|
|
||
| The instructions for deploying the renku application into K8s are the same as in the client, so see that section for those details. There are some small differences in how the server `run-telepresence.sh` script works compared to the client. | ||
|
|
||
| * In the server, `run-telepresence.sh` will prompt you to provide or override the deployment you want to intercept. You can enter the id of a PR to intercept the deployment associated with a GitHub PR. |
There was a problem hiding this comment.
I'm tempted to adjust the 2 telepresence scripts to unify this behavior.
If it's ok with you, once the PR is ready I would add a commit to align it (and update this section accordingly) .
There was a problem hiding this comment.
In general, I'm ok with that change, but I think it would be nicer if the prompting only happened if no values were provided. That is, if there is no DEV_NAMESPACE or PR environment variable set, then prompt for it, but one of them is set, do not prompt and just use what was provided.
There was a problem hiding this comment.
Good suggestion! I will go that way 👍
There was a problem hiding this comment.
Great documentation 👏 I find the added information very clear and useful.
In addition, have you considered including a Support/Troubleshooting section for common errors for example when we are working with telepresence and have to deal with dropping connections.
Should the Administrator's Guide work for all external contributions or do they need additional permissions?
Good point! I added a section about fixing telepresence problems. Is there anything else that you think should be in there?
My understanding is that if you have access to a K8s cluster and have sufficient permissions, you should be able to deploy the renku app using the instructions there. External contributors will need to organize their own K8s cluster, though, since they will not have access to ours. |
Developer documentation
Fix #2165