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

Improve error message logging on exit #592

Merged
merged 14 commits into from Jun 29, 2016
Merged

Improve error message logging on exit #592

merged 14 commits into from Jun 29, 2016

Conversation

tsimonq2
Copy link
Contributor

@tsimonq2 tsimonq2 commented Jun 22, 2016 •
edited

This should fix bug 1574190 due to a suggestion by @ElOpio. This reverts a one-line change made with the commit for cleanbuild.

@snappy-m-o
Copy link

Can one of the admins verify this patch?

1 similar comment
@snappy-m-o
Copy link

Can one of the admins verify this patch?

@tsimonq2
Copy link
Contributor Author

Needs more work, I'll hack on it more

@tsimonq2
Copy link
Contributor Author

Hmm, the unit tests are failing, but I'm not entirely sure that it's because of tweaks I'm making. @ElOpio, thoughts on why this is failing?

come-maiz
come-maiz reviewed Jun 23, 2016
View reviewed changes
snapcraft/main.py

logger.error(textwrap.fill(str(e)))
sys.exit(1)
sys.exit(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bad merge. I changed these lines after you made your branch.
The correct merged statements would be:
logger.error(str(e))
sys.exit(1)

@tsimonq2 tsimonq2 changed the title [Trivial] Fix for bug 1574190 Fix for bug 1574190 Jun 23, 2016
@snappy-m-o
Copy link

Can one of the admins verify this patch?

1 similar comment
@snappy-m-o
Copy link

Can one of the admins verify this patch?

@tsimonq2
Copy link
Contributor Author

Well, this looks like the tests have passed and the bug seems to be fixed.

@ElOpio can you confirm that this is ready?

@kyrofa
Copy link
Contributor

kyrofa commented Jun 28, 2016

Hey @tsimonq2 this turns into a changelog entry. Would you mind selecting a more informative PR title?

@tsimonq2 tsimonq2 changed the title Fix for bug 1574190 Removes error message truncation (bug 1574190) Jun 28, 2016
@tsimonq2
Copy link
Contributor Author

@kyrofa How does that look?

@sergiusens sergiusens changed the title Removes error message truncation (bug 1574190) Improve error message logging Jun 28, 2016
@sergiusens
Copy link
Collaborator

@tsimonq2 I hope you don't mind my change

@sergiusens sergiusens changed the title Improve error message logging Improve error message logging on exit Jun 28, 2016
@tsimonq2
Copy link
Contributor Author

Totally fine @sergiusens

@sergiusens
Copy link
Collaborator

👍

@come-maiz
Copy link
Contributor

👍 Thanks!

@sergiusens sergiusens merged commit f861908 into canonical:master Jun 29, 2016
@tsimonq2 tsimonq2 deleted the bug1574190 branch July 10, 2016 06:02
kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017
@tsimonq2 @sergiusens
Improve error message logging on exit (canonical#592)
4c7543f 
LP: #1574190
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tsimonq2 @snappy-m-o @kyrofa @sergiusens @come-maiz