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

send pre/post code context for exceptions and errors #76

Merged
merged 4 commits into from
Apr 22, 2016

Conversation

modethirteen
Copy link
Contributor

Adds lines of code before and after the "code" line, as described in https://rollbar.com/docs/api/items_post/. We've been using this @MindTouch for a couple years with good results.

I didn't write a test as it uses file() to load source files into arrays. If you think a test is necessary I can create an abstraction for file() that is mockable.

@Crisfole
Copy link
Contributor

Hey @modethirteen You're the man! This all looks really great.

If you don't mind writing a test we'd really appreciate it! I realize this involves refactoring the file reading a bit. Let me know if you need anything from us. We'd be happy to offer you any assistance you need.

@modethirteen
Copy link
Contributor Author

@Crisfole no problem, i'll have the update shortly

@modethirteen
Copy link
Contributor Author

@Crisfole updated and tests added

@Crisfole
Copy link
Contributor

@modethirteen Nice! Thanks for that. I'll inspect later today, and get this merged in ASAP.

@modethirteen
Copy link
Contributor Author

@Crisfole sounds great -- does a new tag get cut after that? it would be great it we were running the mainline releases.

@nocive
Copy link
Contributor

nocive commented Apr 12, 2016

@Crisfole any chance this will get merged soon?

@Crisfole
Copy link
Contributor

@modethirteen @nocive I'd like to merge this soon. Tagging and releasing would need at least one extra set of eyes.

@Crisfole
Copy link
Contributor

@brianr Take a peek? This looks good to me and it's tested.

@Crisfole
Copy link
Contributor

Testing this locally right now and then I'll merge

@Crisfole
Copy link
Contributor

Took a bunch longer because I discovered we didn't actually show the pre/post code yet, but had to make sure. This is fanstastic. Thank so much @modethirteen

@Crisfole Crisfole merged commit 19a37f4 into rollbar:master Apr 22, 2016
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