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

Increase code coverage on bootstrap levels #57

Merged
merged 1 commit into from
Jan 2, 2014
Merged

Increase code coverage on bootstrap levels #57

merged 1 commit into from
Jan 2, 2014

Conversation

jcsmorais
Copy link
Member

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes

// FIXME: need to translate sugar db types to doctrine db types
// we're currently only supporting mysql
if ($config['dbconfig']['db_type'] === 'mysql') {
$config['dbconfig']['db_type'] = 'pdo_mysql';
Copy link
Member

Choose a reason for hiding this comment

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

The list of available ones is here:
http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/configuration.html#driver

Also, we know the ones that Sugar supports, so we can easily do a mapping and have support for them all, since this is only to confirm connection. DB2 isn't yet fully supported by Doctrine, but people are already working on fixing bugs (doctrine/dbal#447) on latest versions so I think we can bet on this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I know that, though I would rather postpone this and move other features forward instead.

What I mean is, we're mostly working with mysql so it is easier to add support for it first since we can test it right away. In my opinion, if down the road there's a need to add support for others we can do it on a separate PR while testing those properly as well.

Agree?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 02d2a20 on jcsmorais:bootstrap-levels-code-coverage into 2e33175 on insulin:master.

alias-mac added a commit that referenced this pull request Jan 2, 2014
Increase code coverage on bootstrap levels
@alias-mac alias-mac merged commit cae274d into insulin:master Jan 2, 2014
@alias-mac alias-mac deleted the bootstrap-levels-code-coverage branch January 2, 2014 01:02
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

3 participants