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

Prettier error formatting upon error exit from supervisor #615

Merged
merged 1 commit into from
Jun 2, 2016

Conversation

kmacgugan
Copy link

Previously supervisor errors looked like this:
SupError { err: HandlebarsTemplateFileError(TemplateError(UnexpectedClosingBraces(5, 44))), logkey: "ER", file: "src/error.rs", line: 300, column: 8 }

Now they look like this:
hab-sup(ER)[src/error.rs:288:8]: Cannot find a release of package in any sources: cargo/foo

Signed-off-by: kmacgugan [email protected]

@thesentinels
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @adamhjk, @metadave, @fnichol and @reset to be potential reviewers

@@ -338,7 +338,7 @@ fn main() {
/// Exit with an error message and the right status code
#[allow(dead_code)]
fn exit_with(e: SupError, code: i32) {
println!("{:?}", e);
println!("{}", e.to_string());
Copy link
Collaborator

@reset reset Jun 2, 2016

Choose a reason for hiding this comment

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

There's actually no need to alloc the SupError into a String here. Simply changing from {:?} to {} is the exact right thing to do 😄. The println! macro will take care of coercing whatever you pass it by calling the fmt::Display trait that SupError is implementing.

@reset
Copy link
Collaborator

reset commented Jun 2, 2016

@kmacgugan thank you!

gif-keyboard-18165966100571083900

@thesentinels r+

@thesentinels
Copy link
Contributor

📌 Commit deb28e0 has been approved by reset

@thesentinels
Copy link
Contributor

⌛ Testing commit deb28e0 with merge 2f0f020...

thesentinels pushed a commit that referenced this pull request Jun 2, 2016
Signed-off-by: kmacgugan <[email protected]>

Pull request: #615
Approved by: reset
@thesentinels
Copy link
Contributor

☀️ Test successful - travis

@thesentinels thesentinels merged commit deb28e0 into habitat-sh:master Jun 2, 2016
jtimberman pushed a commit that referenced this pull request Jun 12, 2016
Signed-off-by: kmacgugan <[email protected]>

Pull request: #615
Approved by: reset
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