-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add neo4j driver on params instance #373
Conversation
dynastymasra
commented
Apr 13, 2020
•
edited
Loading
edited
- Add neo4j driver on the instance param
- Check version constraint avoid duplicate error
- Remove unnecessary field on config struct
Pull Request Test Coverage Report for Build 750
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Having a neo4j.Driver
as a param to WithInstance()
makes sense.
This also means that options that can be specified via the neo4j.Driver
should be removed from Config
. e.g. AuthToken
and URL
I'm fine w/ this backwards incompatible change since it's pretty minor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like tests are failing:
TestMigrate/neo4j:3.5: neo4j_test.go:97: database returned error [Neo.ClientError.Statement.SyntaxError]: Unknown procedure output: `name` (line 1, column 29 (offset: 28))
"CALL db.constraints() yield name WHERE name="schema_migration_constraint_key" RETURN *"
^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linter failed in tests. You need to run gofmt
:
database/neo4j/neo4j.go:23: File is not `gofmt`-ed with `-s` (gofmt)
const DefaultMigrationsLabel = "SchemaMigration"
The command "golangci-lint run" exited with 1.
Also, not sure why tests are failing for neo4j v4. Didn't see anything in the logs...
https://travis-ci.com/github/golang-migrate/migrate/jobs/319479345
--- FAIL: Test/neo4j:4.0 (84.71s)
Hai @dhui |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for the PR and for addressing all of the feedback!