OpenLiberty / guide-cloud-openshift-operator

A guide on how to deploy microservices to Red Hat OpenShift by using Kubernetes Operators.
https://openliberty.io/guides/cloud-openshift-operator.html
Other
2 stars 3 forks source link

Laura's content review feedback #21

Closed lauracowen closed 4 years ago

lauracowen commented 4 years ago

In general, this looks really good. I've provided a bunch of feedback and changes below but the main two things that I think need improving a little are:

Here's the detail:

What you'll learn section

Additional prereqs

Getting started

Installing section

Deploying Kafka

Deploying system and inventory apps

Scaling up section

MaiHameed commented 4 years ago

What you've got isn't far off but I think it needs some slight changes to draw the focus more strongly on what is important to understand to learn anything from this guide. I think the guide is assuming the reader knows Kubernetes but not OpenShift? Is that about right?

The focus of the guide is to show users how to use the Open Liberty Operator. It's not necessarily a focus on OpenShift, but more so that the Operator can streamline and add functionality to deployed OL apps.

A link to OpenShift is useful though so, instead, make the word "OpenShift" at the start of the paragraph into a link to either the docs or just to openshift.io.

I updated the hyperlink to go to openshift.com

Don't mention OKD - it's not relevant to this guide. The point here is that you can use the Open Liberty Operator to deploy and manage OL apps on OpenShift clusters.

The concept of the guide is to introduce Operators, and I think it's important to note that they're also deployable to OKD clusters as well as base Kubernetes clusters in the guide upfront instead of relying on them to navigate to the OL Operator page which states this.

But also move this sentence to above the command. I know there are some includes used here but there's not much point providing an essential instruction after they've run the command, so see how it reads if you move that sentence to the start of the Getting started section: "Run the following command, and all subsequent commands in this guide, from inside your OpenShift cluster."

Minor detail here, in the adoc, the Getting Started block is a pre-written insert taken from here: https://github.com/OpenLiberty/guides-common/blob/master/gitclone.adoc. I've re-written the sentence in question to after the Additional Prereqs section, but before the Getting Started header for this reason. Hopefully this works, let me know if you'd still like this changed, if so, I'll need to manually copy paste the contents of the insert.

yeekangc commented 4 years ago

I would say it's both OpenShift and Operators. I don't believe we are assuming that users have either knowledge or need prior knowledge of one or the other before they try the guide. And, this may become the guide for deploying onto OpenShift over time.

lauracowen commented 4 years ago

Don't mention OKD - it's not relevant to this guide. The point here is that you can use the Open Liberty Operator to deploy and manage OL apps on OpenShift clusters.

The concept of the guide is to introduce Operators, and I think it's important to note that they're also deployable to OKD clusters as well as base Kubernetes clusters in the guide upfront instead of relying on them to navigate to the OL Operator page which states this.

I don't really agree that it's necessary because the guide is teaching you to use the OL Operator - just what's essential to know to use it using this scenario. We have documentation (or will have) that will give you additional info. But if you want to mention OKD here, I can live with that but the guide needs to very briefly explain what it is relative to OpenShift (otherwise it just looks like an acronym that's not been defined).

MaiHameed commented 4 years ago

but the guide needs to very briefly explain what it is relative to OpenShift (otherwise it just looks like an acronym that's not been defined).

The sentence is currently

The Open Liberty Operator provides a method of deploying and managing Open Liberty applications on OpenShift or OKD (the community distribution of OpenShift) clusters.

Do you think OKD needs further explanation or does that look good to you?

yeekangc commented 4 years ago

This is not a guide about Open Liberty Operator. This is a guide for deploying on OpenShift using Open Liberty Operator. See https://github.com/OpenLiberty/guides-common/issues/411.

In my mind, the emphasis should be on OpenShift first and then Open Liberty Operator second. If we need a dedicated guide for Operators, that is a separate issue.

FYI, @gkwan-ibm.

lauracowen commented 4 years ago

Are we assuming they understand the basics of Kubernetes already or not though?

I'll suggests a re-draft of the intro later today but I need to know what existing knowledge we're assuming first.

MaiHameed commented 4 years ago

The green vertical line on the left of the action box is extending lower than the box. Is that correct? It looks a bit odd. Or is that to show that the update is to ad the replicas parameter? In which case, that's fine - it's still a bit odd but it makes more sense (and that's a design thing rather than the guide so don't worry about it).

Yes, the action block is an update action block, which is different from a create action block. The update block has the vertical green line that extends down to include the thing to update. It's a design thing and is shown in other guides as well.

MaiHameed commented 4 years ago

I opened a PR with the changes you requested @lauracowen, feel free to take a look and review the changes thus far. The changes are live on the lgdev site. I'll wait on your re-draft of the intro and change accordingly.

yeekangc commented 4 years ago

Are we assuming they understand the basics of Kubernetes already or not though?

I'll suggests a re-draft of the intro later today but I need to know what existing knowledge we're assuming first.

Basic understanding of Kubernetes is fair. The focus should be aspects specific and relevant to OpenShift than Kubernetes in general IMO. For Kubernetes itself, we can point to the Kubernetes Intro guide.

lauracowen commented 4 years ago

I need to finish a blog post this afternoon so depending on how long that takes, I'll also try to look at the intro again.

lauracowen commented 4 years ago

Okay, based on discussion with @yeekangc and @MaiHameed:

btw, your other changes in the PR look good, thanks. btw2, if you find that the existing OpenShift guide’s instructions are clunky or confusing, flag it up and we can try to fix it rather than perpetuate the problem. That’s fine - we can improve stuff.

MaiHameed commented 4 years ago

23 addressed the additional comments if you could take a quick look @lauracowen. lgdev site has been updated with the latest changes

MaiHameed commented 4 years ago

Addressed with #23