-
Notifications
You must be signed in to change notification settings - Fork 20
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Not sure on the Make file and OpenShift/Make file changes as I haven't look at that code closely
- line 32 (ease installation) - installation is spelled wrong
- "Using the ISO will address " - change to: "Using the prepared ISO addresses the following:" , then list out everything. At the bottom right a note. "NOTE: Optional scripts may be executed if not using the prepared ISO that handle the above prerequisites."
- "for the OpenShift installer - for example" - Change to "for the OpenShift installer. For example,
- Typo spelled "aboave" fix to "above"
- Typo spelled "preformthe" fix to "perform the"
- line 68 can we say "configuring a storage VLAN on the internal interface labeled ???" (I assume it won't be labeled provisioning.
- line 90 - "Most of these items... " change to: "The following items are discovered in a properly configured cluster:"
- Typo line 112 "usinghte" change to : "using the"
- line 113 "subdirectory, Make a copy of" change to "subdirectory, create a copy of
config_example.sh
using the existing user as part of the file name. For example,config_<username>.sh
. Once the file has been created, set the requiredPULL_SECRET
variable within the shell script." - line 116 add ticks to libvirt. Change to
libvirt
--- Libvirt will be installed by ISO so not sure how you want to rewrite all this portion in general.
@rlopez133 you should be able to leave inline feedback in the source view. Try clicking to the left of lines you'd like to comment on when viewing the changes under "Files Changed". |
```sh | ||
make configure | ||
make OpenShift | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add in the README file a note about using a LOGLEVEL environment variable to increase the loglevel output of the openshift-install
command? Such as:
Note: In order to increase the log level ouput of openshift-install
, a LOGLEVEL
environment variable can be used as:
export LOGLEVEL="debug"
make OpenShift
This will fix Russel's comment in #51
I can do a follow up PR if you don't want to add it into this big PR tho :)
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll attempt to get it in prior to @markmc review else may ask you to do the follow up
@@ -1,8 +1,13 @@ | |||
.PHONY: default all OpenShift OCS CNV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script in the CNV dir has changed, so this file needs an update for that. Because of the script names changing, I was thinking a Makefile per directory could make sense, then the root level Makefile just pushd and calls make. So if you raise a PR to change a script name, you need only mess with the Makefile in your directory instead of worrying what other Makefiles call your scripts
…ets in OCS/CNV Makefiles Signed-off-by: Johnny Bieren <[email protected]>
@johnbieren - would you mind given my Makefiel changes a once over |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
fix #26 Feedback on "What's going on here?"
and
fix #50 Outdated README.md instructions
and
fix #51 Allow custom log level