Skip to content

Conversation

@tpoitras
Copy link

Based on content submitted by PR#241 and PR#210.

Related to Trello card: https://trello.com/c/jPzgJkic

Ping @mnagy for Tech Review. Questions for you, Martin:

  1. WRT The command that you provided for testing ( $ mysql -h 127.0.0.1 -uuser -ppass <<< "SELECT 1;" ) are any of those values replaceable by the user? Specifically asking about -uuser and -ppass. Are those supposed to be -u < user > and -p < pass > ? or does the user submit them exactly as you've shown?
  2. Do you think it's worth showing how to build a CentOS-7 based MySQL image, if you can just pull it from Docker?

@mnagy
Copy link

mnagy commented Mar 24, 2015

@tpoitras

  1. Both. The -uuser and -ppass work if you create the container with MYSQL_USER=user and MYSQL_PASSWORD=pass. I wanted them to be easily copy&paste-able, at least in project's README.md. I don't know what's more preferable for docs so you decide, but yes, user and pass can be replaced.
  2. I have no idea, frankly. I based the docs on the readme in the project's github page and there it makes sense. I guess it won't make much sense here, now that I think about it. On the other hand, isn't it weird to include build instructions for rhel and not centos?

The changes LGTM, as far as technical accuracy is concerned.

@adellape adellape mentioned this pull request Mar 25, 2015
@adellape
Copy link
Contributor

@tpoitras Sorry, this'll need a rebase too now, after merging #246.

@tpoitras tpoitras force-pushed the mysql branch 2 times, most recently from 02a2bc6 to bb9628c Compare March 27, 2015 07:15
@tpoitras
Copy link
Author

@mnagy -- can you tech review this latest update? Since we use replaceable text instead of telling them to actually use "user" and "pass", I have updated from -uuser to --user=< user >

Please verify this is still correct. The MySQL docs seems to verify I can use the variant of that command. (http://dev.mysql.com/doc/refman/5.6/en/mysql.html)

@mnagy
Copy link

mnagy commented Mar 27, 2015

@tpoitras yes, this is correct. Though you referenced 5.6 instead of 5.5 :)

One non-technical question/suggestion: We tell user that he can get the mysql image from docker, but we don't mention the image name, except in the later example, but that seems to be hard to see at first glance. Perhaps we should mention somewhere that it's openshift/mysql-55-centos7 or openshift/mysql-55-rhel7? Maybe even elaborate a bit on how we name them generally?

@tpoitras
Copy link
Author

@mnagy -- thanks, I've included some more details on the name of the docker image and how to pull it. If that looks good to you, then I think we are done here. :)

@mnagy
Copy link

mnagy commented Mar 30, 2015

LGTM

@tpoitras
Copy link
Author

@adellape @CowboysFan -- this should be good for merge now

@adellape
Copy link
Contributor

adellape commented Apr 1, 2015

:shipit:

adellape added a commit that referenced this pull request Apr 1, 2015
Changes to MySQL image for style and readability
@adellape adellape merged commit 39be791 into openshift:master Apr 1, 2015
@tpoitras tpoitras deleted the mysql branch April 1, 2015 03:56
sbeskin-redhat pushed a commit to sbeskin-redhat/openshift-docs that referenced this pull request Oct 24, 2022
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.

3 participants