Skip to content

incorporate edits from Bob D #787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 15, 2019
Merged

incorporate edits from Bob D #787

merged 2 commits into from
Jan 15, 2019

Conversation

rosemarymarano
Copy link
Contributor

@rosemarymarano rosemarymarano commented Jan 14, 2019

Including:

  • Noting that the guide assumes a single node cluster.
  • Moved up the instruction to grant the Helm service account the cluster-admin role.
  • Changed the “kubectl log” command to “kubectl logs” (in both files).
  • Added the path for the kubernetes/samples/scripts/create-weblogic-domain/domain-home-in-image/create-domain-inputs.yaml file.

@tbarnes-us
Copy link

tbarnes-us commented Jan 14, 2019

@rosemarymarano @moreaut

@tbarnes-us
Copy link

tbarnes-us commented Jan 14, 2019

Some general comments, indirectly related to this specific change:

  • The file name (helm-charts.md) and doc title ('using operator helm charts') are confusing to me since:
    -- The focus is on installing and running the operator - not helm in particular. Helm is just a tool we use to get the operator running, and is not the sole subject of the guide (for example, it also discusses Elk).
    -- We also need a helm-charts instruction for the load balancers, but the focus of the document is the operator process.
  • So how about
    -- (A) move the bulk of the contents to a new file named something like 'operator.md' with title like 'Managing the Operator'
    -- (B) leave only the helm/tiller install/setup guidance, retitle to 'setting up helm and tiller', and reference this file from from 'operator.md', 'install.md', 'ingress.md', and any other place that has helm install as a pre-req

Also:

  • If removing the operator does not remove the CRD, then the directions for removing the operator in helm-charts.md should be updated to mention this step.

@rosemarymarano
Copy link
Contributor Author

All good suggestions, Tom. I will work on them.

@rjeberhard rjeberhard merged commit a8164ed into 2.0-rc2 Jan 15, 2019
@rosemarymarano rosemarymarano deleted the updateQuickStart branch January 15, 2019 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants