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 slackUsernameFormat config option #171

Merged
merged 3 commits into from
Nov 8, 2016

Conversation

laughinghan
Copy link
Contributor

@laughinghan laughinghan commented Nov 7, 2016

Close #56

@laughinghan laughinghan changed the title Close #56: Add slackUsernameFormat config option Add slackUsernameFormat config option Nov 7, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 99.457% when pulling 4fb44ed on laughinghan:slackUsernameFormat into d35d7b9 on ekmartin:master.

1 similar comment
@coveralls
Copy link

coveralls commented Nov 7, 2016

Coverage Status

Coverage increased (+0.003%) to 99.457% when pulling 4fb44ed on laughinghan:slackUsernameFormat into d35d7b9 on ekmartin:master.

@laughinghan
Copy link
Contributor Author

@ekmartin: lol did you add Coveralls twice or something

@ekmartin
Copy link
Owner

ekmartin commented Nov 7, 2016

Brilliant! I agree with what you said in #56 (comment) - let's go with $username (IRC) as the default. Whether it's a breaking change or not is up for discussion, but I think it's fine to just do it in a minor version bump, since as you said - it's a cosmetic change.

Should this maybe be nested under an object, to make it cleaner to add support for IRC username formatting as well?

"usernameFormat": {
   "slack": "$username (IRC)",
   "irc": "<$username>"
}

Or do you think it's clearer with just slackUsernameFormat and ircUsernameFormat?

EDIT: No idea what's happening with coveralls, let's see if it's a one-time incident ¯\_(ツ)_/¯

@laughinghan
Copy link
Contributor Author

Great! Will do.

I don't think it should be nested, but I also see no use for ircUsernameFormat since messages in IRC from the bot (piped from Slack) are already distinguished by coming from the bot, much moreso than bot messages in Slack.

@ekmartin
Copy link
Owner

ekmartin commented Nov 7, 2016

I think one use case would be if people for some reason would want to remove the username when messages are posted to IRC. Might be good to have both just for unity, but we can merge this and look at it later.

@laughinghan
Copy link
Contributor Author

@ekmartin: eh yeah let's just deal with that later. Updated default Slack username format.

@coveralls
Copy link

coveralls commented Nov 8, 2016

Coverage Status

Coverage increased (+0.003%) to 99.457% when pulling 2334800 on laughinghan:slackUsernameFormat into d35d7b9 on ekmartin:master.

@ekmartin ekmartin merged commit da4fdfe into ekmartin:master Nov 8, 2016
@ekmartin
Copy link
Owner

ekmartin commented Nov 8, 2016

Great, thanks! I'll create a release later.

@ekmartin
Copy link
Owner

ekmartin commented Nov 8, 2016

Released in 3.9.0 - thanks again!

@laughinghan laughinghan deleted the slackUsernameFormat branch November 8, 2016 17:56
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