Skip to content
New issue

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

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

Already on GitHub? # to your account

Refactored MysqlEngine, created PdoEngine, added Postgres support #41

Merged
merged 6 commits into from
Jun 9, 2016
Merged

Refactored MysqlEngine, created PdoEngine, added Postgres support #41

merged 6 commits into from
Jun 9, 2016

Conversation

robertscherer
Copy link
Contributor

In reference to #30

@coveralls
Copy link

coveralls commented May 30, 2016

Coverage Status

Coverage decreased (-5.4%) to 78.005% when pulling d86c28d on robertscherer:master into 4d91e4b on josegonzalez:master.

@robertscherer
Copy link
Contributor Author

@josegonzalez The PdoEngine refactoring and Postgres support is done, would be great if you could give it a look.

I'm not sure what to do about the coveralls error - because effectively, test coverage didn't decrease.

}

/**
* @covers josegonzalez\Queuesadilla\Engine\PdoEngine::connect
Copy link
Owner

Choose a reason for hiding this comment

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

I think the reason test coverage went down is that the MysqlEngine is no longer covered here.

@coveralls
Copy link

coveralls commented Jun 9, 2016

Coverage Status

Coverage decreased (-1.2%) to 82.24% when pulling 833251c on robertscherer:master into 4d91e4b on josegonzalez:master.

@coveralls
Copy link

coveralls commented Jun 9, 2016

Coverage Status

Coverage increased (+1.4%) to 84.795% when pulling d199b42 on robertscherer:master into 4d91e4b on josegonzalez:master.

@robertscherer
Copy link
Contributor Author

Here we go :)

@@ -229,22 +229,17 @@ public function quoteIdentifier($identifier)
return $this->startQuote . $identifier . $this->endQuote;
}

// string.string
Copy link
Owner

Choose a reason for hiding this comment

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

Why not instead pull the regex out into a variable? That should help explain what the regex is for.

@josegonzalez josegonzalez merged commit b329768 into josegonzalez:master Jun 9, 2016
@josegonzalez
Copy link
Owner

Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants