rubenv / sql-migrate

SQL schema migration tool for Go.
MIT License
3.23k stars 280 forks source link

Add custom TxError to add clarity to tx-related error messages. #24

Closed daved closed 8 years ago

daved commented 8 years ago

This PR will improve the clarity of transaction-related errors by associating messages with a particular migration id. No additional tests have been added at this time.

For example, this:

ERROR: syntax error at or near "blah" (SQLSTATE 42601)

will now produce:

ERROR: syntax error at or near "blah" (SQLSTATE 42601) handling 0000_some_filename.sql

go test passes, but I will need direction from @rubenv regarding whether or not additional testing is desirable and, if so, what is the preference for the design/approach of such testing.

rubenv commented 8 years ago

I like this, but it's probably a bit verbose.

How about adding a newTxError(migration, err) function:

func newTxError(migration *PlannedMigration, err error) error {
    return &TxError{
        Migration: migration.Migration,
        Err:       err,
    }
}

This would allow you to replace all these blocks:

            err := &TxError{
                Migration: migration.Migration,
                Err:       err,
            }

            return applied, err

By this:

return applied, newTxError(migration, err)

Which keeps the code much more compact, less duplicated and more readable.

daved commented 8 years ago

Thanks for the correction; This is done.

rubenv commented 8 years ago

Ok, looking good for me. Merging!

Probably also need to backport this to https://github.com/rubenv/modl-migrate

rubenv commented 8 years ago

Oh and thanks a lot, always a joy to land excellent pull requests!