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

Refactor error handling #30384

Merged
merged 5 commits into from
Dec 18, 2015
Merged

Refactor error handling #30384

merged 5 commits into from
Dec 18, 2015

Conversation

nrc
Copy link
Member

@nrc nrc commented Dec 15, 2015

Should make it possible to add JSON or HTML errors. Also tidies up a lot.

@nrc
Copy link
Member Author

nrc commented Dec 15, 2015

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

Travis is still unhappy but I can't quite see why. Still, this looks fine -- mostly just shuffling things around, from what I can tell. r=me once travis errors are resolved.

I feel like the layering of emitters/handlers/etc always feels more complex (or maybe just less clear) than it seems like it needs to be, but that's certainly pre-existing.

@nrc
Copy link
Member Author

nrc commented Dec 15, 2015

The idea (aiui) is that to implement a new way of dealing with errors, you just have to implement Emitter (which is just a single emit method, really). Handler presents a much wider interface to users (span_err, span_warn, etc.). I guess this is kind of convenient, but it won't really work with structured errors, so the next step is probably to make all of those default methods on the Emitter trait, at which point there is not much left to Handler and it can probably go.

@nrc
Copy link
Member Author

nrc commented Dec 15, 2015

@bors: r=@nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 15, 2015

📌 Commit 0f1ae7b has been approved by @nikomatsakis

@nrc
Copy link
Member Author

nrc commented Dec 15, 2015

@bors: r=@nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 15, 2015

📌 Commit 0f1ae7b has been approved by @nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 16, 2015

☔ The latest upstream changes (presumably #30206) made this pull request unmergeable. Please resolve the merge conflicts.

@nrc
Copy link
Member Author

nrc commented Dec 17, 2015

@bors: r=@nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 17, 2015

📌 Commit ff0c74f has been approved by @nikomatsakis

Manishearth added a commit to Manishearth/rust that referenced this pull request Dec 18, 2015
Should make it possible to add JSON or HTML errors. Also tidies up a lot.
bors added a commit that referenced this pull request Dec 18, 2015
@bors bors merged commit ff0c74f into rust-lang:master Dec 18, 2015
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.

3 participants