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.

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

located errors: database #124

Closed
wants to merge 3 commits into from

Conversation

da2ce7
Copy link
Contributor

@da2ce7 da2ce7 commented Dec 8, 2022

Related Issue: #176

Depends #177

This is the first in a series of work to add locations to error messages. I have created the located_error.rs class that gathers location from the error call, and make use of it for the database errors.

@da2ce7 da2ce7 force-pushed the database-error-overhall branch from 7d613da to a3549f3 Compare December 22, 2022 03:58
@da2ce7 da2ce7 marked this pull request as ready for review December 22, 2022 05:14
@da2ce7 da2ce7 force-pushed the database-error-overhall branch 3 times, most recently from cbc8ad5 to b695309 Compare February 4, 2023 15:16
@da2ce7 da2ce7 changed the title refactor: database errors located errors: database Feb 4, 2023
@da2ce7
Copy link
Contributor Author

da2ce7 commented Feb 4, 2023

This pull request is Ready to Review.

@da2ce7 da2ce7 mentioned this pull request Feb 4, 2023
1 task
@da2ce7 da2ce7 force-pushed the database-error-overhall branch from b695309 to 9a3ba06 Compare February 5, 2023 09:14
Comment on lines +44 to +49
let details = format!(
r#"
status: ´{response_status}´
headers: ´{response_headers:?}´
text: ´"{response_text}"´"#
);
Copy link
Member

Choose a reason for hiding this comment

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

hey @da2ce7 , this is an interesting way to have a kind of "object literal" or "anonymous struct". It's related to #172

I would not include it in this PR unless we change all the asserts to follow the same pattern. Maybe the final solution could be a mix of this solution and what I proposed on that PR. In this solution, I still miss the error pointing to the origin line when the condition was not fulfilled.

@josecelano
Copy link
Member

I think this is a very helpful improvement.

ack 9a3ba06

@da2ce7
Copy link
Contributor Author

da2ce7 commented Feb 7, 2023

closing in favor of #178

@da2ce7 da2ce7 closed this Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants