Bootstrap proposal: convert Figures 3-8 to PlantUML, add make target to generate #1058
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/test pull-cluster-api-test |
k8s-ci-robot
added
the
approved
|
| # | ||
| .PHONY: figures | ||
|
|
||
| figures: *.png plantuml.jar |
There was a problem hiding this comment.
we should really be doing this via containers IMO.
There was a problem hiding this comment.
I tried existing public containers. None of them rendered all the characters.
I know I can make it work with a different base image and the newest PlantUML version. Should I publish my own container and reference it here?
There was a problem hiding this comment.
Let's talk with google folks to see if we can get some build containers in place.
/cc @BenTheElder @spiffxp
There was a problem hiding this comment.
i'm not sure where this image would be hosted, I'm still waiting on wg-k8s-infra to give arbitrary repos official hosting, keys etc. kind is still quietly using a docker-hub 🙃
There was a problem hiding this comment.
I'll need to figure out a base image with a font that renders the checkmarks.
There was a problem hiding this comment.
I created my own image, based on the Dockerfile of the official plantuml-server image.
The checkmark (✅) codepoint did not render correctly because a font was missing. I found out which font(s) were capable of rendering this codepoint with this helpful script.
I installed fonts-symbola to the image, and the checkmark rendered correctly.
I am hosting the image on Docker hub. The Dockerfile for my image is https://github.com/dlipovetsky/plantuml-docker.
There was a problem hiding this comment.
/uncc
I'm fine with the dockerhub container for now, if you want to host on gcr.io open a PR against kuberentes/k8s.io, ref: kubernetes/k8s.io#158 (comment)
|
(I'll squash once we've agreed on all the details) |
|
/test pull-cluster-api-test |
dlipovetsky
force-pushed
the
states-preboot-proposal
branch
from
c7c19d6 to
42bc75e
Compare
|
Squashed. |
k8s-ci-robot
added
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dlipovetsky, timothysc, vincepri The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test pull-cluster-api-test |
dlipovetsky
force-pushed
the
states-preboot-proposal
branch
from
42bc75e to
ab15bcd
Compare
k8s-ci-robot
removed
the
lgtm
|
Rebased due to conflict with |
|
/test pull-cluster-api-test |
|
machine deployment reconcile test failed /retest |
|
/hold cancel |
k8s-ci-robot
added
lgtm
What this PR does / why we need it:
Removes dependency on external service to generate sequence diagrams in #997. Allows sequence diagrams to be generated locally.
Special notes for your reviewer:
I tried to use an upstream containerized PlantUML client, but none of them rendered the UTF-8 characters (checkmarks) correctly, so I kept the native java client.
/cc @vincepri