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

Add option for Linkifying user name and channel name #22

Conversation

tomoima525
Copy link
Contributor

Hi! Thanks for the great service! I added a small feature to slack message step.

Done in this PR

  • Added option to the step to linkify user name and channel name when posted from slack bot

screen shot 2017-08-01 at 22 54 52

this is related issue #13

Copy link
Contributor

@viktorbenei viktorbenei left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the contribution @tomoima525 ! :)

I left a few comments for discussion, but I'm happy with the PR as it is, the code is 👍

step.yml Outdated
@@ -151,6 +151,16 @@ inputs:
leave this option empty then the default one will be used.
is_expand: true
is_required: false
- link_names: "no"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we enable this by default? Seems useful enough to be default yes IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that is true! addressed bdff5e3

step.yml Outdated
opts:
title: "Link Names"
description: |
Link names such as @username
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also add a channel example here. E.g.

Linkify names in the message such as `@slackbot` or `#random`

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, should include channel name too -> 779e002

@tomoima525
Copy link
Contributor Author

@viktorbenei thanks for the comments, I made fix to them

main.go Outdated
@@ -31,6 +31,7 @@ type ConfigsModel struct {
EmojiOnError string
IconURL string
IconURLOnError string
IsLinkNamesOn bool
Copy link
Contributor

Choose a reason for hiding this comment

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

one last minor thing, please rename this property to just IsLinkNames, to conform to the naming of the other properties

@viktorbenei
Copy link
Contributor

Thank you @tomoima525 ! :) Found one last thing, sorry I missed this the first time.

@tomoima525
Copy link
Contributor Author

@viktorbenei Sure thing! Just addressed

Copy link
Contributor

@viktorbenei viktorbenei left a comment

Choose a reason for hiding this comment

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

Perfect, thank you very much @tomoima525 ! :)

I'll release a new version ASAP!

@viktorbenei viktorbenei merged commit 75c5e8b into bitrise-steplib:master Aug 7, 2017
@viktorbenei
Copy link
Contributor

Released in v2.6.1 - should be available in the steplib soon

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

2 participants