Skip to content
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

Added a minimal run_latest_build.sh with instructions #296

Merged
merged 1 commit into from
Jul 22, 2017
Merged

Conversation

jwmatthews
Copy link
Member

No description provided.

@jwmatthews
Copy link
Member Author

Note this PR requires an update from #295

Copy link
Contributor

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the script to the scripts/ directory with all other scripts.

README.md Outdated
- **latest** - The newest release build
- **<release_number>** - The stable release of an image

# QuickStart - Running Ansible Service Broker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong size heading. Should be ##

README.md Outdated

# QuickStart - Running Ansible Service Broker

## Running
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump to ###

@jwmatthews
Copy link
Member Author

I'm unsure about moving this to scripts.
scripts is all developer focused.

My rationale for moving this to root of the directory was so it stood out as the script to use to run things if you aren't familiar with the developer flow.

Thoughts?

README.md Outdated

## Running

The below will use `oc cluster up` to bring up a cluster with Ansible Service Broker installed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following will use ....

README.md Outdated

1. Ensure that your system is setup to run `oc cluster up`
* Follow [these instructions](https://github.com/openshift/origin/blob/master/docs/cluster_up_down.md) if you haven't already seen success with `oc cluster up`
1. Download [run_latest_build.sh](https://raw.githubusercontent.com/openshift/ansible-service-broker/master/run_latest_build.sh)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the script in the scripts directory not in the root of the project.

README.md Outdated

1. Execute [run_latest_build.sh](https://raw.githubusercontent.com/openshift/ansible-service-broker/master/run_latest_build.sh)
* ./run_latest_build.sh
* Will take ~90 seconds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move the 90 seconds line up to the bullet item, like so:

  1. Execute run_latest_build.sh, this will take ~90 seconds.
    • ./run_latest_build.sh

README.md Outdated
* Will take ~90 seconds
1. You now have a cluster running with the Service Catalog and Ansible Service Broker ready

## Sample Workflow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump down to ###

1. Select 'Create Binding'
1. Select the Postgres service and complete creating the Binding
1. Redeploy mediawiki so the pod is able to consume the credentials for the database.
1. View the route for mediawiki and verify the wiki is up and running.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

steps looks good 👍

README.md Outdated
1. Redeploy mediawiki so the pod is able to consume the credentials for the database.
1. View the route for mediawiki and verify the wiki is up and running.

# Developer Focused Information Below
## Prerequisites
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump Developer Focused ... to ## and Prerequisites to ###

@jmrodri
Copy link
Contributor

jmrodri commented Jul 21, 2017

Script looks good. Just some formatting changes. Excellent work! 👍

@jmrodri
Copy link
Contributor

jmrodri commented Jul 21, 2017

@jwmatthews there isn't any scripts in the root dir. We have readme, makefile, rpm spec, glide. Anyone cloning this repo will likely be trying to develop on it. If they are visiting the repo and reading the documentation, they will see the link to the script living in scripts. Actually they probably won't even notice since they'll just click the link.

We can create a bin/ directory if we want to make it stand out, but honestly I think scripts will be fine. -1 to root.

@jwmatthews
Copy link
Member Author

Moved to scripts directory and updated formatting.

Copy link
Contributor

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@@ -39,7 +39,61 @@ Read more about the Ansible Service Broker and Ansible Playbook Bundles in this
* [Ansible Service Broker - Design](docs/design.md)
* [Other Documentation](docs/README.md)

## Prerequisites
## Published Images
The ansible-service-broker community publishes images in (dockerhub)[https://hub.docker.com/u/ansibleplaybookbundle].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting changes look great. Thanks

# with the parameter DOCKERHUB_ORG being passed into the template.

DOCKERHUB_USER="changeme"
DOCKERHUB_PASS="changeme"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn this bit me forgot to change this before I ran the script :(

@jmrodri jmrodri merged commit c828b1a into master Jul 22, 2017
@jmrodri jmrodri deleted the mindeploy branch July 27, 2017 13:55
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.

None yet

2 participants