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 Mattermost transport with configurable author_name #9759

Merged
merged 1 commit into from Feb 13, 2019

Conversation

Projects
None yet
4 participants
@zombah
Copy link
Contributor

commented Jan 29, 2019

Regarding Mattermost api doc author_name is optional
And no reason to force hostname to author_name because hostname
already exist in alert topic, so make author_name configurable.

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

Update Mattermost transport with configurable author_name
Signed-off-by: Misha Komarovskiy <zombah@gmail.com>
@TheGreatDoc

This comment has been minimized.

Copy link
Contributor

commented Feb 3, 2019

Hi @zombah

Thanks for your PR. As for testing this a Mattermost server is needed, do you mind posting 2 screnshoots, one before and one after the patch?

@zombah

This comment has been minimized.

Copy link
Contributor Author

commented Feb 4, 2019

Current behaviour, triple hostname:
mattermost-with-patch-hostname-author
With patch, configurable author_name is empty:
mattermost-with-patch-empty-author

@murrant
Copy link
Member

left a comment

LGTM

@murrant murrant merged commit fe4bd2b into librenms:master Feb 13, 2019

5 of 6 checks passed

codeclimate 2 issues to fix
Details
Inspection Summary
Details
Node: analysis
Details
Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
license/cla Contributor License Agreement is signed.
Details

@zombah zombah deleted the zombah:transport-mattermost-author_name branch Feb 13, 2019

@gpant

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2019

The reason I had the author name set to the hostname is that it made it a lot easier to get the hostname when using the mattermost API to access the web hook stream. Now for doing the same exact work there is need for regex, having the author name blank or on a static value does not have any benefit.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 22, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.