apetkau / orthomcl-pipeline

Automates running of OrthoMCL software from http://orthomcl.org/common/downloads/software/v2.0/
80 stars 36 forks source link

Adding database creation to orthomcl-setup-database #2

Closed JenCabral closed 9 years ago

apetkau commented 9 years ago

Overall, this looks great. Thanks for taking on this task :).

One suggestion I have for you

  1. Could you modify the README.md and the INSTALL.md files to explain how to setup mysql so it can be used with this new version of the script? I believe the main change is that we have to log into mysql ahead of time and run commands to create an orthomcl user, and grant privileges to this user. Could you write these commands down? The files are written using Markdown which you can read more about at https://help.github.com/articles/markdown-basics/.
apetkau commented 9 years ago

This looks great. One suggestion I have would be but could you add an option --no-create-database to scripts/orthomcl-setup-database.pl? That is, if this option is set, skip over creation of a database and just check for a connection and write a orthomcl.conf file. My reason for this is that, although for our group we always want to create a database, if anyone else uses this software they may not want to build a new database (and setup an orthomcl account with permissions on all databases).

Overall, this looks great though. Thanks.

apetkau commented 9 years ago

Overall, this looks awesome. Thanks :). Only other comment I have is could you add a section in INSTALL.md in Database Setup just describing how a user doesn't have to GRANT SELECT, INSERT, UPDATE, DELETE, CREATE on *.* to orthomcl if they already have a specific database setup for OrthoMCL? That is something like:

Other then that, everything works great. I'll merge it in as soon as you make those updates. Thank you.

JenCabral commented 9 years ago

Updated files as requested.

JenCabral commented 9 years ago

Updated.

JenCabral commented 9 years ago

Updated

apetkau commented 9 years ago

Awesome, this looks great :). Thanks for putting up with all of my requests. Code merged.