Feature/naming-updates #1275
Feature/naming-updates #1275
Conversation
bkneis
requested review from
janekbaraniewski,
pascalbreuninger and
Piotr1215
There was a problem hiding this comment.
@bkneis really nice use of the PlantUML diagrams, I left a few suggestions. In general, once the diagrams' repository will grow, you can take advantage of the preprocessing capabilities and reuse some components either via procedure or function calls.
On the sequence diagrams you can also use themes, I think !theme cloudscape-design would fit devpod pretty well!
bkneis
force-pushed
the
feature/docs
branch
from
e23fc5d
to
f57769e
Compare
There was a problem hiding this comment.
Overall good additions, thanks!
I don't agree on all of the naming changes but that's naming I guess. Commented on the ones I find irritating.
Other than that it's mostly small nitpicks and suggestions.
Could you additionally swap out all devpod mentions for DevPod please? At least our docs should refer to the actual name :D
| sidebar_label: Deploying kubernetes | ||
| --- | ||
|
|
||
| DevPod works the same with kubernetes as with Machines, the key difference is the secure tunnel is set up using the kubernetes control plane (e.g. kubectl ...) so an agent is not necessary |
There was a problem hiding this comment.
so an agent is not necessary
Technically we still have a local agent and one in the workspace container, just not one in the VM. What do you think of removing that part?
There was a problem hiding this comment.
this was the complete sentence - so an agent is not necessary to be run on the kubernetes node. I wanted to add this as looking at machine you could easily think the devpod agent is on the k8s node as well as the pod
There was a problem hiding this comment.
I can remove, but I think it's a good distinction
There was a problem hiding this comment.
@pascalbreuninger, @bkneis you can use vale to ensure DevPod is always spelled with proper capitalization. LMK if you are interested, and I can set it up for you.
|
@pascalbreuninger Just finished updating this PR, all of your comments but one should be addressed, including name changes :) Just one remark on your comment, let me know what you think and if I should remove |
I have added some docblocks and performed some re factoring as I was looking around the code to check if the docs were correct. Some of this was making certain statements more idiomatic but mostly it was renaming. I found a lot of the code had long fully qualified names and duplicate words once the package identifier was included, i.e.
devcontainer.FindDevContainer. I know naming is very subjective so I only changed what I thought was awkward to read.