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

Update report-http.md #704

Merged
merged 5 commits into from
Dec 3, 2021
Merged

Update report-http.md #704

merged 5 commits into from
Dec 3, 2021

Conversation

cadirol
Copy link
Contributor

@cadirol cadirol commented Dec 2, 2021

Hy!
Great work with BGPalerter!
i like to update the report documentation with a RocketChat Http Request
Cheers

Hy!
Great work with BGPalerter!
i like to update the report documentation with a RocketChat Http Request
Cheers
@massimocandela massimocandela changed the base branch from main to dev December 3, 2021 14:56
@massimocandela
Copy link
Member

Thanks for this contribution. I changed the PR to be merged in dev.

I have some remarks (1 is the most important):

  1. Are you sure the "payload=" part should be there? If I look at the documentation it looks like a JSON post body should be enough. Could you please verify that the example you provided works exactly as is (except for user-specific params). Be aware, the initial ' wrapping the payload is not closed. Please, if a string and a JSON post body are available, prefer the JSON one.
  2. Can you describe the role of the "channel" key?
  3. Are you ok with me adding "Thanks to @cadirol" at the bottom of the example?

Thanks

@cadirol
Copy link
Contributor Author

cadirol commented Dec 3, 2021 via email

@massimocandela
Copy link
Member

I applied my review. Let me know if you are ok with it and if everything looks fine. Then, I will merge it.

@cadirol
Copy link
Contributor Author

cadirol commented Dec 3, 2021

Seems good! your additional hint for #channel and @user completes the howto

@massimocandela massimocandela merged commit e6b106f into nttgin:dev Dec 3, 2021
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.

2 participants