Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Feature/error messages #1327

Merged
merged 8 commits into from Nov 19, 2022
Merged

Feature/error messages #1327

merged 8 commits into from Nov 19, 2022

Conversation

jantimon
Copy link
Contributor

@jantimon jantimon commented Sep 9, 2022 •
edited by dsherret

This is a POC branch to improve error messages in two ways:

  1. Allow custom error messages Providing custom messages to OrThrow methods #715
  2. Show the source location of the error

error

To finish this PR I would need feedback on the approach itself and your help to improve errors.ts.

Closes #715

@jantimon jantimon mentioned this pull request Sep 12, 2022
Closed
dsherret
dsherret requested changes Sep 30, 2022
View reviewed changes
Copy link
Owner

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

@jantimon sorry for my delay! This looks really good. Thanks! Would you be able to add a few tests for the more complicated functionality where it prints out the message? Then we should be good to merge this.

packages/common/src/errors.ts Outdated Show resolved Hide resolved
@jantimon
Copy link
Contributor Author

jantimon commented Oct 2, 2022 •
edited

no worries - I am happy you like the idea :)

@jantimon
Copy link
Contributor Author

@dsherret if it helps you to save some time on this issue we can also discuss this PR in a video call

dsherret
dsherret approved these changes Nov 19, 2022
View reviewed changes
Copy link
Owner

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

Did a few changes and it LGTM now. Thanks for this @jantimon!

@dsherret dsherret merged commit 05916d3 into dsherret:latest Nov 19, 2022
@jantimon
Copy link
Contributor Author

wow nice! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsherret dsherret dsherret approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Providing custom messages to OrThrow methods
2 participants
@jantimon @dsherret