Resolves #647 - Fixes typos in Mongo advisory locking parameters #648
Conversation
Pull Request Test Coverage Report for Build 1424405064
💛 - Coveralls |
database/mongodb/mongodb.go
Outdated
|
|
||
| if lockTimeout == "" { | ||
| // The initial release had a typo for this argument but for backwards compatibility sake, we will keep supporting it. | ||
| lockTimeout = unknown.Get("x-advisory-lock-timout-interval") |
There was a problem hiding this comment.
Thanks for preserving the old behavior! Let's return an error explaining that only one should be specified if both are specified.
There was a problem hiding this comment.
Is that really important. I am on vacation for a few days, but the only way that would happen is if someone looked in the source code to see this behavior as we aren't documenting both, so I'm not sure if it is the end of the world if we just use the correct one.
There was a problem hiding this comment.
This fix isn't urgent so can wait until you're back from vacation. Or I can make the change (based off of your change). I'd like to support existing users in a backwards compatible manner but also eliminate any confusion if both parameters are specified.
CONTRIBUTING.md
Outdated
| @@ -11,7 +11,7 @@ | |||
| 1. Write awesome code ... | |||
| 1. `make test` to run all tests against all database versions | |||
| 1. Push code and open Pull Request | |||
|
|
|||
| :wq | |||
database/mongodb/mongodb.go
Outdated
| ErrNilConfig = fmt.Errorf("no config") | ||
| ErrNoDatabaseName = fmt.Errorf("no database name") | ||
| ErrNilConfig = fmt.Errorf("no config") | ||
| ErrTypoAndNotNonTypoUsed = fmt.Errorf("both x-advisory-lock-timeout-interval and x-advisory-lock-timout-interval were specified") |
There was a problem hiding this comment.
Since this is an exported specific error, let's make the name specific as well. e.g. something along the lines of: ErrLockTimeoutConfigConflict
* 'master' of github.com:golang-migrate/migrate: (418 commits) Run gofmt on internal build dir go mod tidy Resolves golang-migrate#647 - Fixes typos in Mongo advisory locking parameters (golang-migrate#648) Bump version of autorest/adal Set syntax highlighting for pkger example Add pkger to README change github auth to use oauth token instead of basic. Use the recommended v4 in mysql README go mod tidy fix test Delete all rows Use ParseBool Support for AWS Keyspaces Update gosnowflake from v1.4.3 to v1.6.3 Update docker client usage with breaking change Update dktest to v0.3.7 to fix security warnings revert binary file location change in docker image fix source/file driver Update build constraints Update golangci-lint config ...
make-testwas failing a bunch for me on neo4j, and mssql, not sure if it's just my environment however. I can investigate if you think it's a risk.This is for #647