Hyphen in user name problem
marekrozmus opened this issue · 9 comments
@all-contributors please add aaa-bbb for bug, ideas
Will add user aaa not a aaa-bbb user.
Hmm! Thanks for reporting this bug.
Would you mind opening a PR to fix this?
@Berkmann18:
Firat of all sorry for reporting bug in incorrect repository - I've just found that there is separate repository for bot and cli 😢
Now about reported issue: I've checked again with your bot tests and I'm not sure if this is bug at all.
When I used
@all-contributors please add @aaa-bbb for bug, ideas then it worked correctly - it added user aaa-bbb (with hyphen).
So my question would be rather why prefixing username with @ works?
Please decide and close this issue if needed. Thanks 😄
I've put up a pull request to add @aaa-bbb! 🎉
Firat of all sorry for reporting bug in incorrect repository - I've just found that there is separate repository for bot and cli
No worries.
Now about reported issue: I've checked again with your bot tests and I'm not sure if this is bug at all.
When I used
@all-contributors please add @aaa-bbb for bug, ideas then it worked correctly - it added user aaa-bbb (with hyphen).
So my question would be rather why prefixing username with @ works?
That's because the bot requires the use of @ in the request, it previously accepted comments such as "Please add someUser for ..." but the code got changed for it to only accept @someUser.
As for the CLI, it doesn't require that, but a fix would be welcome.
I could not determine your intention.
Basic usage: @all-contributors please add @jakebolam for code, doc and infra
For other usages see the documentation
bot is listening 😉 Anyway in the documentation this information about the @ prefix is missing. It is only visible on screenshot that is why I got confused.
Hmm, good catch. Feel free to submit a PR for this if you can.
If not, I'll do it but I'm very busy at the moment.
I believe we can also close this one now with all-contributors/app#307, given that this actually relates to the bot repo? (The test suite now also covers this specific use case)
@baikho Yup, correct.