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

integrations: added honeybadger webhook #14281

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

integrations: added honeybadger webhook #14281

wants to merge 2 commits into from

Conversation

Udit107710
Copy link
Collaborator

Testing Plan:
Tested it manually using the dev env and integration tool

@Udit107710 Udit107710 force-pushed the master branch 2 times, most recently from 764235c to df81c3b Compare March 21, 2020 16:36
@Udit107710
Copy link
Collaborator Author

Kindly review.
All the specified changes has been taken into account.

punchagan
punchagan suggested changes Mar 25, 2020
View reviewed changes
Copy link
Member

@punchagan punchagan left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @Udit107710. Looks good to me, apart from the one minor comment I left.

zerver/webhooks/honeybadger/view.py Outdated Show resolved Hide resolved
@Udit107710
Copy link
Collaborator Author

@punchagan
Made the changes as suggested.

@Udit107710 Udit107710 force-pushed the master branch 2 times, most recently from df49bf9 to c9a3f88 Compare March 25, 2020 12:30
@Udit107710
Copy link
Collaborator Author

@timabbott
Could you review the changes?
I have integrated the suggestions.

@punchagan punchagan mentioned this pull request Mar 30, 2020
Closed
@Udit107710
Copy link
Collaborator Author

@timabbott
Any more changes?

@timabbott
Copy link
Sponsor Member

Can you work on improving these templates? I feel like after reading the tests, they don't seem like a very nice way to present the information contained in the payloads.

For example, I think a lot of your links could be in markdown, and given that very long messages are possible, it seems like those should often be in a paragraph by themself.

@Udit107710 Udit107710 force-pushed the master branch 2 times, most recently from ec8eba3 to 45093bd Compare April 21, 2020 11:28
@Udit107710
integrations: Added honeybadger webhook.
674d611 
Honeybadger mintors web applications to report various events like
errors or deploymnet. Added support for 9 out of 10 events supported
by Honeybadger webhook integration. Tested the integration manually
on my dev env. Fixtures were captured using ngrok.
@Udit107710
images: Optimised svgs.
3080bee 
Including honeybadger.svg, various other svgs were also optimised
using svgo tool.
@Udit107710
Copy link
Collaborator Author

Udit107710 commented Apr 21, 2020 •
edited

@timabbott
I have added Linking and Emphasis in the templates. Regarding the large messages, a large message only arrives in the case when a comment is made. I have handled that case differently and put the message bode in a different paragraph. All other payload do no contain a message long enough for a different paragraph.

@Udit107710
Copy link
Collaborator Author

@timabbott Any more issues?

@zulipbot
Copy link
Member

zulipbot commented Sep 8, 2021

Heads up @Udit107710, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

@timabbott timabbott added the completion candidate PRs with reviews that may unblock merging label Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@punchagan punchagan punchagan approved these changes

@timabbott timabbott Awaiting requested review from timabbott

Assignees
No one assigned
Labels
completion candidate PRs with reviews that may unblock merging has conflicts size: XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Udit107710 @timabbott @zulipbot @punchagan