Content review

[evelinec]

Intro

• seems like the RestClientBuilder method should go first.. Agree that it's not an issue

Try what you’ll build

• The http://localhost:9080/inventory/systems/localhost microservice retrieves the system property information for a specific host named localhost by invoking the http://localhost:9080/system/properties client
  => a period is missing at the end of the bullet
  => seems like the http://localhost:9080/system/properties URL is being referred to as a client here, where as in the bullet above, it’s a microservice. I think it’s be good to refer to the http://localhost:9080/system/properties URL as the system service (or microservice) first, then in later text, you can say the system service.

• In addition, try to get your internal IP address, and visit the http://localhost:9080/inventory/systems/{your_ip_address} URL. You will see the same system property information, but the process of getting this information is actually implemented differently.
  => Only understand it after going through the whole guide. Not sure we need it in this section. If keeping it, perhaps it'd good to have it in another bullet too?

Writing the RESTful client interface

• This added dependency provides the library that your code requires to implement the MicroProfile Rest Client interface.
  => The word “added” doesn’t seem necessary
  => I don’t like the mention of “your code”, but it’s optional to make a change

• The mpRestClient-1.0 feature is also enabled in the src/main/liberty/config/server.xml file. This feature allows you to use the MicroProfile Rest Client to invoke RESTful services over HTTP in a type-safe way.
  => still don’t see why user need this very clearly. perhaps remove “over HTTP in a type-safe way” because it draws away the importance of using the MP Rest Client to invoke RESTful services. Or make it more specific that this feature is the implementation of the API in Open Liberty; then user needs it for the application to run.

• It simulates a remote RESTful service that the inventory service tries to invoke.
  => I think it is not a simulation. It simply is the remote RESTful service the the inventory service tries to invoke. Agree that it's not an issue

• Now, create a RESTful client interface that represents the system service, call it the SystemClient. Create the SystemClient class in the src/main/java/io/openliberty/guides/inventory/client/SystemClient.java file:
  => May be good to put it as:
  Now, create a RESTful client interface that represents the system service. To do this, create the SystemClient class in the src/main/java/io/openliberty/guides/inventory/client/SystemClient.java file:
  => the word “represents” doesn’t seem accurate here. how about “for the system service”?

• The @dependent annotation tells the service the scope of the interface.
  => seems necessary to explain more for this annotation, especially to state why this scope annotation is used for this class.

• I thought the “Handling exceptions..” section was to be a subsection under “Writing the RESTful client interface”?

Injecting the client with dependency injection

• 9080 in the InventoryManager class should not be hard coded?

General:

• Clean up the comments like “// Not Found” in the code

• {your_ip_address} can be a hostname other than the “localhost” too. Perhaps change the name to be more generic.

[JunyiSun]

Fixed by #29 #36

[evelinec]

Under "Try what you'll build", I think for the 3 URLs, you can refer to them with their service names. Just listing them as URLs defeats the purposes, ie, not sure why we want have the http://localhost:9080/system/properties URL for a specific localhost. The way you listed the URLs in the "Building and running the application" section seems much clearer.

In the "Try what you'll build" section, it'd good to refer to the results (ie, URLs) back to the services that were introduced in the "What you'll learn" section.

[evelinec]

Still think this one can be made clearer that the feature in the server.xml file relates to OL.

The mpRestClient-1.0 feature is also enabled in the src/main/liberty/config/server.xml file. This feature allows you to use the MicroProfile Rest Client to invoke RESTful services over HTTP in a type-safe way.
=> still don’t see why user need this very clearly. perhaps remove “over HTTP in a type-safe way” because it draws away the importance of using the MP Rest Client to invoke RESTful services. Or make it more specific that this feature is the implementation of the API in Open Liberty; then user needs it for the application to run.