Skip to content
This repository has been archived by the owner on Jun 28, 2018. It is now read-only.

Don't load all drivers #54

Merged
merged 13 commits into from
Nov 11, 2015
Merged

Conversation

gravis
Copy link
Contributor

@gravis gravis commented Oct 15, 2015

This PR also includes fixed tests

lepoetemaudit and others added 4 commits June 11, 2015 11:11
Requires activating drivers with a _ style import, e.g.
import "_ github.com/mattes/migrate/driver/postgres"
@gravis gravis mentioned this pull request Oct 15, 2015
@gravis
Copy link
Contributor Author

gravis commented Oct 15, 2015

Final step before merging, in .travis.yml:

go:
  - 1.3
  - 1.4
  - 1.5
  - tip

While we're using only golang:latest docker image. I'm looking how to fix that. Ideas are welcome

@gravis
Copy link
Contributor Author

gravis commented Oct 15, 2015

There's no "tip" golang image, I have removed it, please let me know if this is a blocking issue.

@gravis gravis changed the title Dont load all drivers 43 Don't load all drivers Oct 15, 2015
blankDriver := reflect.New(reflect.TypeOf(driver)).Interface()
d, ok := blankDriver.(Driver)
if !ok {
err := errors.New(fmt.Sprintf("Driver '%s' does not implement the Driver interface"))

Choose a reason for hiding this comment

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

This should be:

err := fmt.Errorf("Driver '%s' does not implement the Driver interface", u.Scheme)

(missing interpolation variable)

@gravis
Copy link
Contributor Author

gravis commented Oct 19, 2015

btw, refs #43

We can't test driver.New methods because of circular imports.
That's probably why the initial code had a map[string]interface{} as
registry: it removes a dependency import.
I prefer the remove the registry and have a registry returning a real
Driver. It will ease the development later.
@gravis
Copy link
Contributor Author

gravis commented Oct 22, 2015

Tests are failing with go 1.3 because of goql (cassandra driver): https://github.com/gocql/gocql/blob/master/control.go#L13

atomic.Value was introduced in go 1.4.

This is not related to this PR. I suggest to drop go 1.3 support.
Using vendoring in a lib sounds overkill to me.

@mattes
Copy link
Owner

mattes commented Oct 22, 2015

I'm fine with dropping 1.3.

mattes added a commit that referenced this pull request Nov 11, 2015
@mattes mattes merged commit 2a2bc4f into mattes:master Nov 11, 2015
@mattes
Copy link
Owner

mattes commented Nov 11, 2015

Merged! Awesome work! Thanks @gravis

@gravis gravis deleted the dont-load-all-drivers-43 branch January 24, 2017 20:31
@coveralls
Copy link

coveralls commented Aug 4, 2017

Coverage Status

Changes Unknown when pulling 65674ac on gemnasium:dont-load-all-drivers-43 into ** on mattes:master**.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants