Peer Review

[NimG98]

Peer Review: review to be done by a peer member.

• [x] Check that the guide has working instructions and sample code
• [x] Check the quality of code according to the best coding practices
• [x] Check the quality and presentation of guide according to Structure and Style Guideline
• [x] Check the consistency of guide with template and other guides
• [x] Examples with the right outcomes are provided
• [x] Example with the wrong outcomes, if any, are provided
• [x] Check that all licensing statements are properly stated in all files
• [ ] Check that the pom.xml, server.xml, etc files are clean
• [x] Check that the directories are properly structured
• [x] Test the guide on 3 platforms: Mac, Win, Linux
• [x] If any URL visits, try them on different browsers: Firefox, Chrome, Safari
• [x] If any URL visits, try curl command where applicable
• [ ] Ensure that the guide is using the latest version of the liberty-maven-plugin or liberty-gradle-plugin where applicable
• [x] Check that some of these page-tags are used in a guide: MicroProfile, Maven, Docker, Kubernetes, Gradle, Java EE, Security, Cloud. Only these tags are visible on the website. Latest list here.
• [x] Check the attribution statement is accurate for the guide
• [x] Verify the travisTest.sh script, if any, is accurate and consistent with other guides

Peer Testing: tests to be done by a peer member.

Guide’s contributor’s (if available, otherwise peer tester’s) responsibility:

• [x] Additional tests where applicable:
  • [x] Define test coverage and review with team (including guide contributor, if available)
  • [x] Define detail test cases
  • [x] Consider corner cases targeting the specific guide
  • [x] Consider different platform tests, ie, Mac, Windows, Linux
  • [x] Consider corner cases UI tests
  • [x] Consider testing URL on all browsers, ie, FF, Chrome, Safari
  • [x] Consider testing the curl command for URL visits
• [x] Consider building with both Maven and Gradle build tools
• [x] Testing with different IDEs, ie, Atom, Eclipse (Optional: VS.code, IntelliJ, Microclimate)
• [ ] Run Acrolinx Checker on draft (above 70 score approximately)
• [x] Consider SEO title and description for the guides
• [x] Ensure automated test is enable, set up with Travis CI, and able to schedule tests
• [x] Run diff -r start/ finish/ and there's no differences
• [ ] Ensure that the automation tests are able to run when PR is created

Peer Tester’s responsibility:

• [x] Check the appearance of the guide on test site for the following items:
  • [x] Table of contents
  • [x] Headings
  • [x] Paragraphs
  • [x] code snippets
  • [x] outputs
  • [x] links
  • [x] hotspots
• [x] Test the guide end-to-end with working instruction and sample code
• [x] Perform all the defined test cases

[NimG98]

Building the system microservice

• [x] Add backticks to start -> start

  Navigate to the start directory.

• [ ] Maybe separate the Navigate to the start directory. line and make the next part a separate paragraph

Logging in to the cluster

• [x] Change the following sentence :

  Minishift already includes the oc binary, to use the oc commands, run the following command to configure your PATH to include the binary

  To make it two sentences:

  "Minishift already includes the oc binary. To use..."

• [x] output of minishift oc-env command for mac has linux output:

Building the Docker image

• [x] maybe say here which Docker image you're building (i.e. the system image):

  Now that the environment is set up, ensure that you are in the start directory and run the following command to build a Docker image:

Deploying the system microservice

• [x] Add backticks to "system" - > system

  Now that the system Docker image is built, deploy it using a resource configuration file.

• [ ] hotspots in yaml file are highlighting extra blank lines below

• [x] Change RUNNING to Running to match the command output:

  Ensure that your system-deployment pod is RUNNING:

• [ ] code on sidepane hides the appropriate things, but if you click the file header on the codepane, it stops hiding those parts and shows the entire file

• [x] Not sure if www.mywebapp.com is needed:

  Now you have to expose the service using a route so that external users can access the microservice through a host name, such as www.mywebapp.com.

• [x] output of oc get routes is displaying service where it should say route

  • output should be something like:

• [x] change "host name" -> "hostname"

• [x] change host-name -> hostname

Deploying the inventory microservice

• [x] say what you're pushing your image to (like in the previous section for system: "Now push your newly tagged image to the internal registry with the following command")

  Run the following command to push your microservice:

• [x] mention again that the image link is found in the DOCKER REPO column of the image stream from the oc get imagestream command. This is so they don't have to scroll up again since they forgot the command:

  Update the configuration file to add the inventory resources, making sure to update the inventory image field with the appropriate image link.

Testing the microservices

• [ ] pom.xml hotspots (system.ip and inventory.ip) aren't highlighting anything
• [x] for the windows command, inconsistency between using host-name in earlier sections and using host-ip in this section

  [system-route-host-ip] and [inventory-route-host-ip]

• [x] says {kube} in pom.xml property description, should be "Kubernetes". This was probably the variable {kube}, but the variable itself was deleted from the readme.
• [x] mvn verify command substitution for Mac does not work properly:

  command not found: -Dsystem.ip=system-route-nim-project.192.168.64.2.nip.io

  • it's treating each line as a separate command, and hence it fails.

• [ ] mvn verify command \ is messing up when you click the copy button and paste it into the terminal
  • the first part of the command mvn verify -Ddockerfile.skip=true is automatically run, without the rest of the command

[NimG98]

• [ ] add guide to draft automation tests

Note:

• tested travisTest.sh and it is failing locally. The travis environment has openshift project as myproject. When i run the travisTest locally, it build's docker image using the specified oc project, which in my case is nim-project: docker build -t `oc registry info`/`oc project -q`/system:test system/. So it ends up becoming docker build -t 172.30.1.1:5000/nim-project/system:test system/. But the test.yaml has myproject as the project name, and hence the failure.

• Maybe make the oc project name in test.yaml match oc project -q, or remember to make sure the travis environment never changes the openshift project, otherwise it will cause failures

[NimG98]

End-to-end test:

• Mac (GOOD)
• Linux (GOOD)
• Windows (GOOD)

[NimG98]

• [ ] license years are off
  • a lot of the files look like they were taken from old guides, and are either the same or were modified
  • if they were never changed, they should have that old year/year-range: e.g.201X
  • if they were modified, then it should say e.g. 201X, 2019

[NimG98]

• [ ] need to use:
  • ideal pom
  • latest LMP (3.1)

[NimG98]

• [ ] on Mac, when you copy the curl command, and paste it in the terminal (default terminal, vscode) it looks like: curl http://\[host-name\]/system/properties instead of curl http://[host-name]/system/properties. It's fine for copying and pasting into other applications in Mac though (such as browsers, text files)

[MaiHameed]

• [ ] license years are off

  • a lot of the files look like they were taken from old guides, and are either the same or were modified
  • if they were never changed, they should have that old year/year-range: e.g.201X
  • if they were modified, then it should say e.g. 201X, 2019

Already talked to Gilbert about this, since it's a new guide, it isn't wrong to have all the licence years at 2019, even if they're identical to applications from other guides.

[MaiHameed]

• [ ] need to use:

  • ideal pom
  • latest LMP (3.1)

Addressing in upcoming PR, from mai/simple-pom branch

[MaiHameed]

• [ ] add guide to draft automation tests

Note:

• tested travisTest.sh and it is failing locally. The travis environment has openshift project as myproject. When i run the travisTest locally, it build's docker image using the specified oc project, which in my case is nim-project: docker build -t `oc registry info`/`oc project -q`/system:test system/. So it ends up becoming docker build -t 172.30.1.1:5000/nim-project/system:test system/. But the test.yaml has myproject as the project name, and hence the failure.
• Maybe make the oc project name in test.yaml match oc project -q, or remember to make sure the travis environment never changes the openshift project, otherwise it will cause failures

Regarding this, when Travis starts a test, it'll automatically use the myproject project since I did not specify for it to create a specific project name, therefore the test.yaml file is already configured to use the myproject name. I already tested and ensured everything worked before merging the CI tests in a previous PR, so if you retry the script locally with the myproject project name, everything will (or should) run as expected with no errors.

[MaiHameed]

hotspots in yaml file are highlighting extra blank lines below

UI issue, they're already aware, I can't fix it.

[MaiHameed]

Issues that weren't addressed were moved to #10, everything else in here was addressed, closing.