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

Trim contents of slack api urls from files #2929

Merged
merged 2 commits into from Sep 9, 2022

Conversation

srhb
Copy link
Contributor

@srhb srhb commented May 23, 2022

Trailing newlines will almost always be present in a posixish file, so trim it away at first opportunity to prevent having invalid URLs we won't discover until a notification is sent.

Copy link
Member

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

LGTM

@gotjosh
Copy link
Member

gotjosh commented Jun 13, 2022

Can you please fix the DCO and create a test case for this? Thanks!

@srhb
Copy link
Contributor Author

srhb commented Jul 29, 2022

Sure, here you go 😸

Signed-off-by: Sarah Brofeldt <sarah@qtr.dk>
@simonpasquier
Copy link
Member

you'd need to fix the test

notify/slack/slack_test.go:88:12: undeclared name: `ioutil` (typecheck)
	f, err := ioutil.TempFile("", "slack_test_newline")
	          ^
make: *** [Makefile.common:198: common-lint] Error 1

Exited with code exit status 2

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@simonpasquier simonpasquier merged commit 565d73d into prometheus:main Sep 9, 2022
@simonpasquier
Copy link
Member

thanks!

qinxx108 pushed a commit to qinxx108/alertmanager that referenced this pull request Dec 13, 2022
* Trim contents of slack api urls from files

Signed-off-by: Sarah Brofeldt <sarah@qtr.dk>

* notify/slack: fix test imports

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Signed-off-by: Sarah Brofeldt <sarah@qtr.dk>
Signed-off-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Signed-off-by: Yijie Qin <qinyijie@amazon.com>
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.

None yet

3 participants