Skip to content

Support any data to be passed by attach (replace attach_message) #609

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
Merged
TimDiekmann merged 27 commits into from
Jun 6, 2022
Merged

Support any data to be passed by attach (replace attach_message) #609

TimDiekmann merged 27 commits into from
Jun 6, 2022

Conversation

@TimDiekmann
Copy link
Member

@TimDiekmann TimDiekmann commented Jun 3, 2022 •
edited
Loading

🌟 What is the purpose of this PR?

For convenience, we want to attach any data without implementing a trait.

🔗 Related links

🔍 What does this change?

  • Rename attach_message to attach
  • Support any displayable data in attach_message

⚠️ Known issues

  • Until Error is moved to core, we need to work around that in the doc string

📹 Demo

let error = fs::read_to_string("config.txt")
    .report()
    .attach(Suggestion("Better use a file which exists next time!"));

Alfred-Mountfield
Alfred-Mountfield reviewed Jun 3, 2022
View reviewed changes
@TimDiekmann
Copy link
Member Author

TimDiekmann commented Jun 3, 2022

As discussed off-GitHub, we will combine attach_message and provide. Marking this PR as a Draft and closing the reviews for now.

@TimDiekmann TimDiekmann marked this pull request as draft June 3, 2022 13:26
@TimDiekmann TimDiekmann changed the title Add Report::provide method Support any data to be passed by attach (replace attach_message) Jun 3, 2022
@TimDiekmann
Merge branch 'main' into td/provide-directly
74b6b3b 
# Conflicts:
#	packages/engine/bin/cli/src/main.rs
#	packages/engine/bin/hash_engine/src/main.rs
#	packages/engine/lib/error/src/lib.rs
#	packages/engine/lib/error/src/macros.rs
@TimDiekmann TimDiekmann marked this pull request as ready for review June 3, 2022 13:56
Alfred-Mountfield
Alfred-Mountfield reviewed Jun 3, 2022
View reviewed changes
Copy link
Contributor

@Alfred-Mountfield Alfred-Mountfield left a comment

Choose a reason for hiding this comment

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

I'm a big fan of how much this has simplified things.

Can you look through the comments for references to old things like "message", "provider" (in the wrong places), etc.

TimDiekmann and others added 6 commits June 3, 2022 17:42
@TimDiekmann
Update packages/engine/lib/error/src/frame.rs
18d2b9c 
Co-authored-by: Alfred Mountfield <am@hash.ai>
@TimDiekmann
Update packages/engine/lib/error/src/frame.rs
e9473b5 
Co-authored-by: Alfred Mountfield <am@hash.ai>
@TimDiekmann
Update packages/engine/lib/error/src/report.rs
6fc1721 
Co-authored-by: Alfred Mountfield <am@hash.ai>
@TimDiekmann
Update packages/engine/lib/error/src/result.rs
31a543b 
Co-authored-by: Alfred Mountfield <am@hash.ai>
@TimDiekmann
Simplify Frame implementation by removing Unerased
725a303
@TimDiekmann
Clean up code
3277be6
Alfred-Mountfield
Alfred-Mountfield reviewed Jun 3, 2022
View reviewed changes
@TimDiekmann TimDiekmann enabled auto-merge (squash) June 6, 2022 09:51
@TimDiekmann TimDiekmann merged commit 649eab4 into main Jun 6, 2022
@TimDiekmann TimDiekmann deleted the td/provide-directly branch June 6, 2022 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Alfred-Mountfield Alfred-Mountfield Alfred-Mountfield approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TimDiekmann @Alfred-Mountfield