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

Updating ServerDB.cpp to check for MySQL, SQLite, and Postgres #3039

Merged
merged 1 commit into from Apr 21, 2017

Conversation

Projects
None yet
2 participants
@lewisca04
Contributor

lewisca04 commented Apr 18, 2017

As a fix to issue #1269

#1269

Updated ServerDB::ServerDB() with a check for MySQL, SQLite, and Postgres

@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Apr 18, 2017

Member

We also need to check for Postgres, since we support that now.

Member

mkrautz commented Apr 18, 2017

We also need to check for Postgres, since we support that now.

@lewisca04

This comment has been minimized.

Show comment
Hide comment
@lewisca04

lewisca04 Apr 18, 2017

Contributor

The branch should be updated to account for Postgres.

Contributor

lewisca04 commented Apr 18, 2017

The branch should be updated to account for Postgres.

Show outdated Hide outdated src/murmur/ServerDB.cpp
@@ -80,6 +80,9 @@ void ServerDB::loadOrSetupMetaPKBDF2IterationsCount(QSqlQuery &query) {
}
ServerDB::ServerDB() {
if (Meta::mp.qsDBDriver != QLatin1String("QMYSQL") && Meta::mp.qsDBDriver != QLatin1String("QSQLITE") && Meta::mp.qsDBDriver != QLatin1String("QPSQL")) {
qFatal("ServerDB: invalid DB driver specified: '%s'. Murmur only supports QSQLITE, QMYSQL, and QPSQL.");

This comment has been minimized.

@mkrautz

mkrautz Apr 21, 2017

Member

You're using a %s format string here, but you don't provide any input.

You should use qPrintable(Meta::mp.qsDBDriver)

@mkrautz

mkrautz Apr 21, 2017

Member

You're using a %s format string here, but you don't provide any input.

You should use qPrintable(Meta::mp.qsDBDriver)

@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Apr 21, 2017

Member

Change looks good to me once my suggested fix is in.

Also, could you squash your changes into a single commit, please? And fix PR title + description to include Postgres.

Member

mkrautz commented Apr 21, 2017

Change looks good to me once my suggested fix is in.

Also, could you squash your changes into a single commit, please? And fix PR title + description to include Postgres.

@lewisca04 lewisca04 closed this Apr 21, 2017

@lewisca04 lewisca04 reopened this Apr 21, 2017

@lewisca04 lewisca04 changed the title from Updating ServerDB.cpp to handle MySQL and SQLite check to Updating ServerDB.cpp to check for MySQL, SQLite, and Postgres Apr 21, 2017

@lewisca04

This comment has been minimized.

Show comment
Hide comment
@lewisca04

lewisca04 Apr 21, 2017

Contributor

Hey Mikkel,

I probably didn't go about changing it to one commit in the correct way, but nonetheless the fix should be available with one commit. I changed the PR title, and I assume the description is the first comment. If not, I can change it elsewhere. Thanks for the communication!

Contributor

lewisca04 commented Apr 21, 2017

Hey Mikkel,

I probably didn't go about changing it to one commit in the correct way, but nonetheless the fix should be available with one commit. I changed the PR title, and I assume the description is the first comment. If not, I can change it elsewhere. Thanks for the communication!

@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Apr 21, 2017

Member

Looks good, will wait for the build checks to finish!

Member

mkrautz commented Apr 21, 2017

Looks good, will wait for the build checks to finish!

@mkrautz mkrautz merged commit 65c2500 into mumble-voip:master Apr 21, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lewisca04 lewisca04 deleted the CSCI-462-01-2017:sqlfix branch Apr 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment