Text refinement without fixing the image problem #31
Conversation
|
|
|
@JensHaley it is difficult to quickly see what you actually did change in the text, especially because you reformatted the line breaks. Could you revise that? Btw. I did not see any issues with the images in scenario 2 & 3. Maybe you can show it to me ... |
| Gardener managed shoot cluster resides in the corresponding seed cluster. This is a | ||
| [Control-Plane-as-a-Service](https://kubernetes.io/blog/2018/05/17/gardener/#kubernetes-control-plane) with | ||
| a [network air gap](https://kubernetes.io/blog/2018/05/17/gardener/#network-air-gap). | ||
| ## Goal: Increasing the Security of all Gardener Stakeholders |
There was a problem hiding this comment.
you changed the title.
but the text (except for the emphasis **) is the same. imho the 80 char line breaks were fine.
There was a problem hiding this comment.
Sorry about the line breaks. How can I take back my pull request? I think I need to start all over again if the line breaks are an issue - I see my changes also in Word so I can do it line by line instead of paragraph by paragraph.
imho, using bold text in the text body should be avoided for better readability. The title "Introduction" was quite generic, so my suggestion would be to change the title to reflect the importance of the aspect previously formatted in bold.
There was a problem hiding this comment.
Hi @vasu1124 , the changes should be easier to review now.
There was a problem hiding this comment.
@vasu1124 @freegroup
About the images: Please have a look at the current online version.
Scroll down to section "Scenario 2: ...".
The images contain boxes for "grafana" and "prometheus".
However, this is discussed in scenario 3, not scenario 2.
That's why I assume that the images do not fit and were swapped by mistake.
| architecture, the control plane of a Gardener managed shoot cluster | ||
| resides in the corresponding seed cluster. This is a | ||
| [Control-Plane-as-a-Service](https://kubernetes.io/blog/2018/05/17/gardener/#kubernetes-control-plane) | ||
| with a [network air |
There was a problem hiding this comment.
This line break in the middle of the link is also not easy to read ...
|
Unfortunately, a reasonable "diff" is almost impossible, since line breaks and formatting have been completely rewritten. Things like blank lines in an enumeration and line breaks in a link should be cleaned up before merging this pull request. |
|
|
||
| - Root cause: Same certificate authority (CA) is used for both the | ||
| API server and the proxy that allows accessing the API server. | ||
|
|
There was a problem hiding this comment.
empty line in a enumeration breaks markdown format.
There was a problem hiding this comment.
In the markdown preview of the atom editor I cannot see that anything looks broken. Without empty lines it will be hard to read - could we verify this together? thanks
Please check images of scenario 2 and scenario 3.
|
With @msohn help (thanks again!), I overwrote the original pull request, please check tab Files changed. After that I got a request by @ThormaehlenFred to add 2 sentences (second commit to patch-1 of this pull request). I hope that you can review my changes better now. @freegroup : Please revise the order of the graphics (scenario 2 and scenario 3) yourself, they seem to be mixed up and you probably know better what you wanted to do. Before accepting more review requests, I need to become more familiar with GIT, pull requests, and so on. We don't use github in UA on a regular basis so I need some time to get more familiar with this. Thanks, |
|
|
||
| Those examples cover the following scenarios: | ||
| Alban Crequy [Kinvolk](https://kinvolk.io/) and Dirk Marwinski ([SAP SE](https://www.sap.com)) gave a presentation entitled [Hardening Multi-Cloud Kubernetes Clusters as a Service](https://kccncchina2018english.sched.com/event/H2Hd/hardening-multi-cloud-kubernetes-clusters-as-a-service-dirk-marwinski-sap-se-alban-crequy-kinvolk-gmbh) at KubeCon 2018 in Shanghai presenting some of the findings. |
There was a problem hiding this comment.
Please put Kinvolk in parenthesis as you have done it with SAP
There was a problem hiding this comment.
Thanks Frederik - I added the missing brackets now with my last commit.
I reordered the images as good as I could based on the following draft document: https://docs.google.com/document/d/1LfFFo-saCUbNjWQIePovi7u-IyPRAtYuTbaX6Qj9yHY/edit# Note however, that I didn't have a preview before my commit, because the images are not included using standard MD syntax.
|
@vasu1124 @ThormaehlenFred @freegroup : In my last commit, I corrected the order of the images (thanks to Alban who pointed me to an older draft version of the document). If you still think there are open issues, please contact me. |
|
Unless you would like me to ask a native speaker to do language-editing that should be included in this pull-request. Depends on your deadline and roll-out strategy. |
Note:
Images of scenario 2 and scenario 3 do not fit to the text (not fixed, please check this yourself because I don't know your original intentions).
My keyboard inserts different symbols for " and ' - I guess that this is a difference between Windows and Mac.
I changed "mitigations" to "measures", because a mitigation does not solve the issue, it is a workaround to make the issue smaller.