If no branch is defined, use an empty string. #29
Conversation
|
Hi @annawinkler! Thank you so much for the PR. How are you looking up the revision? Are you deploying a revision directly? |
|
Hi @parkr - great question! When a branch is not specified, the |
|
I'm using Capistrano 2 if that helps... |
|
Ah interesting. It sounds like you had to work around a That said, I’d be happy to accept this PR in the interest of unblocking you all 😄 |
lib/capistrano/slack_notify.rb
Outdated
| end | ||
|
|
||
| def deploy_target | ||
| [slack_app_name, branch].join('/') + (rev ? " (#{rev[0..5]})" : "") | ||
| [slack_app_name, branch_or_not].join('/') + (rev ? " (#{rev[0..5]})" : "") |
There was a problem hiding this comment.
This will produce slack_app_name/@revision when a branch name isn’t specified. Are you cool with this or would you rather see slack_app_name@revision?
There was a problem hiding this comment.
Oh that's a good point. No, I think seeing slack_app_name@revision would be better. I'll update the PR.
|
Hi @parkr ! I tested this branch with the following 3 different scenarios:
I like your suggestion to include the revision if it is specified, and I like the output for (2) and (3). For (1), in the case where no branch and no revision is specified, What are your thoughts now about this PR? Anything else that would be good to consider or does this look OK to merge? |
|
This is perfect! Thank you so much for your contribution. I’ll merge and release a new version today. 😄 |
|
Thanks @parkr ! 🎉 |
|
1.3.4 has been released! 🎉 |
|
Woo hoo - thanks!!! |
|
Thank you for making this project better ❤️ |
In the project I'm working on, we do not always have branch defined. Without a branch defined, unfortunately, an exception is thrown. An empty string seems like a reasonable value to return if there is no branch defined.