Skip to content

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

Merged
rubenv merged 2 commits intorubenv:masterfrom
daved:add/mig_id_to_err0
Feb 24, 2016
Merged

Add custom TxError to add clarity to tx-related error messages.#24
rubenv merged 2 commits intorubenv:masterfrom
daved:add/mig_id_to_err0

Conversation

@daved
Copy link
Copy Markdown
Contributor

@daved daved commented Feb 23, 2016

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
Copy link
Copy Markdown
Owner

rubenv commented Feb 24, 2016

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
Copy link
Copy Markdown
Contributor Author

daved commented Feb 24, 2016

Thanks for the correction; This is done.

@rubenv
Copy link
Copy Markdown
Owner

rubenv commented Feb 24, 2016

Ok, looking good for me. Merging!

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

rubenv added a commit that referenced this pull request Feb 24, 2016
Add custom TxError to add clarity to tx-related error messages.
@rubenv rubenv merged commit 969836b into rubenv:master Feb 24, 2016
@rubenv
Copy link
Copy Markdown
Owner

rubenv commented Feb 24, 2016

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

@daved daved deleted the add/mig_id_to_err0 branch February 24, 2016 22:19
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.

2 participants