Closed yeekangc closed 4 years ago
I am fine with the guide except for the part where users need to reboot. This needs to be addressed first before we proceed to publish.
FYI, @gkwan-ibm.
"You can run through the development cycle using OpenShift’s intuitive web console through the URL provided in the command output of the minishift start command. The web console provides a user friendly GUI alternative to their CLI tools, however this guide will continue with the CLI tools. You can also run the following command to open the web console": If we won't ask the users to use the console, what's the point of including this section? Let us revise and/or elaborate? Is it for them to try it out on their own?
My reasoning for including this paragraph is to highlight the availability of the web console as it is preferred for those who are not as comfortable with using the CLI. The web console however is a difficult resource to write a guide on due to how quickly it can update vs a more robust CLI tool. I rewrote the paragraph to the following:
You can run through the development cycle using OpenShift's web console through the URL provided in the command output of the
minishift start
command. The web console provides a user friendly GUI alternative to their CLI tools that you can explore on your own. This guide will continue with the CLI tools. You can also run the following command to open the web console:
Let me know if it's suitable, otherwise I can update it further.
"The provided deploy.yaml configuration file outlines a deployment resource which will create and deploy a container that will run the Docker-formatted image provided in the image field": Should be "... deploy a system-container container ..."? This makes it easier to map from the description to what is highlighted in the YAML too?
I suppose technically it is a container labelled system-container
so I rewrote the phrase as
... resource which will create and deploy a container named
system-container
that will ...
Let me know if that's sufficient, or if you would rather use
... resource which will create and deploy a
system-container
container that will ..
Re web console: As long as we make it clear that it's an option that users can explore on their own, we should be fine.
"... container named system-container ..." sounds better.
Perhaps ID can review now too and we can do one last review after all edits are in.
"You will see the following response, note that it has been properly formatted in this guide for easier readability": What is the purpose of the "readability" clause?
The default response is not properly formatted. Similar to the conversation we had about how the system properties response is all in one line and not formatted as a typical JSON object would be, due to the need for a web extension to do the formatting. I added this clause to address the question that the user might have as to why their response would look different from the output of the guide.
May be something like "you will see a JSON response like the following. Note that your JSON response may not be formatted. The sample output has been formatted for readability".
It's worthwhile to have ID review this paragraph too.
Would you like to review all the updates once I'm finished them, or do I request for an ID review straight away after the updates are live on the draft site?
For the Tearing down the environment
section, would it be fair to direct the users to the official docs on how to Uninstall Minishift? Or would you prefer to keep a paraphrased version of those steps and explanations in the guide?
If we have not done it elsewhere and if it's not critical to the guide, we can refer to official documentation to remove Minishift. Cleaning up the deployment itself that is covered by the guide is fair.
I would prefer to look at the final updates after you have all the edits for the above (i.e. from this issue) and from ID review incorporated.
Addressed, closing
"You will learn how to deploy two simple microservices with Open Liberty to an OKD cluster running in Minishift": Spell out OKD again?
"Deploying microservices on an OKD cluster using Minishift": Should it be "deploy to" or "deploy on"?
"OKD, formerly known as OpenShift Origin, is the upsteam open-source project for all OpenShift products. OKD is a Kubernetes-based platform with added functionality. If you would like to learn more about Kubernetes, check out the Deploying microservices to Kubernetes guide. OKD streamlines the DevOps process by providing an intuitive development pipeline. It also provides integration with multiple tools to make the deployment and management of cloud applications easier. To learn more about the different platforms that Red Hat OpenShift offers, check out the official OpenShift documentation. To learn how to deploy microservices to OpenShift Online, check out the Deploying microservices to OpenShift guide.": Consider doing all the explanation before inserting the links to make the paragraph more readable?
"You will use Minishift, an all-in-one virtual machine built on top of Minikube": I find this definition somewhat odd. We can keep it simple and say that Minishift enables developers to work with OKD easily in their development environments? Or, that it's for developers to work with a single-node OKD cluster for development purposes?
"Similar to how Minikube makes it easy to deploy a local Kubernetes cluster locally, Minishift allows a quick and easy environment for application development that supports all OKD features": Again, I think we can make this simpler. Also, for "local Kubernetes cluster locally", we have a redundant "local"?
"For installation instructions, refer to the official OKD Minishift documentation": Either add a line in the beginning of the section to say that you need to have these tools installed or change this sentence to say something like "Install OKD Minishift by following the instructions in its documentation". For former is more consistent with how it was done for the other guides.
"The resulting output differ based on your OS and environment, but you will get an output similar to the following": "Differs" instead of "differ"?
"You can run through the development cycle using OpenShift’s intuitive web console through the URL provided in the command output of the minishift start command": I don't think we need "intuitive" here? =)
"You can run through the development cycle using OpenShift’s intuitive web console through the URL provided in the command output of the minishift start command. The web console provides a user friendly GUI alternative to their CLI tools, however this guide will continue with the CLI tools. You can also run the following command to open the web console": If we won't ask the users to use the console, what's the point of including this section? Let us revise and/or elaborate? Is it for them to try it out on their own?
"The source code of the system and inventory microservices are located ...": Should we highlight "system" and "inventory" to be consistent with the rest of the guide?
"The resulting compiled code will be packaged into a war web archive which can be found at system/target/system.war": Should be "... will be package into a system web archive that can be found under the system/target directory"?
"The archive contains all the configuration files, as well as the application needed to run the microservice on an Open Liberty server, and is now ready to be injected into a Docker container for deployment": Does the war actually contain all configuration files?
"Because the Windows command prompt doesn’t support the command substitution that is displayed for Mac and Linux, run the following commands": Do we need to repeat this multiple times?
"The provided deploy.yaml configuration file outlines a deployment resource which will create and deploy a container that will run the Docker-formatted image provided in the image field": Should be "... deploy a system-container container ..."? This makes it easier to map from the description to what is highlighted in the YAML too?
"The image is the name and tag of the container image that you want to use for the system container. Update the system image field to the image link found in the DOCKER REPO column of the image stream": I find "image", "system image field" and "image link" somewhat confusing to understand and parse.
The image field (or key) specifies the name and tag of the container image you want to use for the system container. Update the value of the image field (or key) to specify the image location found under the DOCKER REPO column from the output of ...
"The events log is useful in debugging any issues that may arise": Where does this "event log" come from?
Why do we need to update the deploy.yaml multiple times?
"Update the configuration file to add the inventory resources, making sure to update the inventory image field with the appropriate image link found in the DOCKER REPO column of the oc get imagestream command": Why won't we repeat the oc get imagestream command here as a copyable command?
"You will see the following response, note that it has been properly formatted in this guide for easier readability": What is the purpose of the "readability" clause?
"The default properties that are defined in the pom.xml file are": We should explain the purpose of these properties?
"Substitute [system-route-hostname] and [inventory-route-hostname] with the appropriate values found by running the oc get routes command": Should we repeat "oc get routes" as a copyable command?
Are system.ip and inventory.ip parameters or properties?
"To completely delete your Minishift VM, cluster, and all associated files run the following command": What do we mean by "all associated files"?
"To revert your Docker daemon back to its default settings, simply reboot your system": This is concerning and undesirable in most cases. Do we have to reuse the Docker daemon then?
Switching between platforms always moves me back to the beginning of the section. If this is only for the draft environment, it's alright though it's annoying. I didn't see it in production. If this persists, we should file an issue to have it looked at.
Be sure to have ID review for the guide.