-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Include directory in slack webhook message #662
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- tests are failing, looks like you need to change what are the expected calls on the mock
- can you provide a screenshot of what the notification looks like now
Codecov Report
@@ Coverage Diff @@
## master #662 +/- ##
=========================================
+ Coverage 72.2% 72.2% +<.01%
=========================================
Files 61 61
Lines 4644 4652 +8
=========================================
+ Hits 3353 3359 +6
- Misses 1045 1046 +1
- Partials 246 247 +1
Continue to review full report at Codecov.
|
I think it's okay to always print the directory because it would be weird if you had
And it only printed the directory for The directory is relative and if it's in the root, it will just be |
Updated to make |
@@ -98,6 +98,11 @@ func (d *DefaultSlackClient) createAttachments(applyResult ApplyResult) []slack. | |||
} | |||
|
|||
text := fmt.Sprintf("Apply %s for <%s|%s>", successWord, applyResult.Pull.URL, applyResult.Repo.FullName) | |||
directory := applyResult.Directory | |||
if directory == "." { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if directory == "." { | |
// Since "." looks weird, replace it with "/" to make it clear this is the root. | |
if directory == "." { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one small change please. Then looks good.
I realized I could make the edit myself, thanks! |
Thanks for merging! |
Fixes: #660