Feedback #4 to address from the playback

steven1046 commented 6 years ago

Background concepts

[x] Java CDI is not a thing, drop the word Java because the actual name is Context Dependency Injection.

Devops Pipeline example:

[x] Add description of the example saying that there are 4 ports on 1 application running on 1 Liberty
[x] Show a diagram with the different ports and what values of cars will be returned from each like on the whiteboard. --> Pic was sent to Murphy; Initial draft due 12/5 to us. --> This should replace the words on the Example step, but may have to be careful that with accessibility it can still be read correctly. --> Implemented as an SVG
[x] 'For performance and security reasons...' -> 'For stability and security reasons....'
[x] capitalize the 'd' on displayCarTypes ... it's not correct on one of them.

Injecting @ConfigProperty

[x] Verify the stack trace is correct after getting a working version of the code. Is jboss really included in the stack trace? --> Trace was verified with Annie. Michal to double check when he has a working sample application going.

Providing default values with injection step:

[x] Improve the url domain to not be a potential car site, it can just be 'localhost' --> Michal to verify with legal that we can use 'mycarvendor.openliberty.io'
[x] Do not use 'foo' and 'bar', use actual car types
[x] Add to the instruction which values are going to be expected from the development port. Something like 'Because this request is for our development server, we are expecting back the values of....' within the green instruction.
[x] Do not use sedan, it is not valid in the U.K.

Configuring with properties file step:

[x] If there are multiple microprofile-config.properties files, how does Open Liberty decide which to use? --> Sent a note to Emily on 12/4. Copied Michal and Dana as requested. --> See note below in another comment on this topic --> Emily's response is posted below. The value used if set in more than one microprofile-config.properties file is indeterminate. --> A paragraph will be added to the configure with properties file page.
[x] Add to the instruction which values are going to be expected from the development port.

Configuring with an environment variable step:

[x] Copy button is the same as reset https://github.com/OpenLiberty/iguides-common/pull/75
[x] Take sedan out of the car list (not fully translatable - UK uses saloon)
[x] Add to the instruction which values are going to be expected from the development port.
[x] You can also set an environment variable on the command line. Information on this should be included.

Configuring using a system property step:

[x] Remove note and incorporate it into the paragraph above it.
[x] Do not mention that bootstrap.properties does not exist until you create it, because none of the files exist by default.
[x] Add to the instruction which values are going to be expected from the development port.

Changing ordinal step:

[x] In the instruction, mention that 500 is the highest value because the user might not remember that 400 was the highest used by default.

Overall....

[x] Work with the static guide owners to use the same application between the two guides. We will only have the port property to set. They should include the port property in their sample app, even if they don't talk about it. --> Michal is currently working on the sample application.
[x] Should we show an image on each configure step to illustrate where the values are coming from? --> Murphy provided a simple image which we are going to try out directly following the instructions. --> The review on 12/8 resulted in no further image needed. Updates to the status bar on the web page was sufficient.
[x] Should a method showing that the port is being used be part of the example in the editor?
[x] 'music-download' still appears in our callback.js. Remove code if not in use or correct the comments.
[x] Get the editor and the browser to be the same size. Watch that the size of the editor outside of a tabbedEditor has the filename header preprended to the editor widget!

steven1046 commented 6 years ago

The copy button fix was implemented in https://github.com/OpenLiberty/iguides-common/pull/75

steven1046 commented 6 years ago

If you are working on one of these, add your name next to it so no one else picks it up.

dmorgant commented 6 years ago

Emily's response to having a property assigned a value in more than one microprofile-config.properties file, both on the classpath.....

"If the same property is specified in more than one microprofile-config.properties files, which file supplies the value is undetermined. In practice, the same config property should not occur in more than one microprofile-config.properties. If you want to determine its value, you should override it via system property or environment variables. Thanks Emily"

steven1046 commented 6 years ago

Fixed editor height in https://github.com/OpenLiberty/iguides-common/pull/76

OpenLiberty / iguide-microprofile-config-intro

Feedback #4 to address from the playback #59