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

Hyphen in user name problem #225

Closed
marekrozmus opened this issue Nov 8, 2019 · 9 comments
Closed

Hyphen in user name problem #225

marekrozmus opened this issue Nov 8, 2019 · 9 comments

Comments

@marekrozmus
Copy link

@all-contributors please add aaa-bbb for bug, ideas

Will add user aaa not a aaa-bbb user.

@Berkmann18
Copy link
Member

Hmm! Thanks for reporting this bug.
Would you mind opening a PR to fix this?

@marekrozmus
Copy link
Author

@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 😄

@allcontributors
Copy link
Contributor

@marekrozmus

I've put up a pull request to add @aaa-bbb! 🎉

@Berkmann18
Copy link
Member

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.

@allcontributors
Copy link
Contributor

@Berkmann18

I could not determine your intention.

Basic usage: @all-contributors please add @jakebolam for code, doc and infra

For other usages see the documentation

@Berkmann18 Berkmann18 transferred this issue from all-contributors/all-contributors Nov 12, 2019
@marekrozmus
Copy link
Author

@Berkmann18

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.

@Berkmann18
Copy link
Member

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.

@baikho
Copy link

baikho commented Apr 28, 2020

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)

@Berkmann18
Copy link
Member

@baikho Yup, correct.

Berkmann18 pushed a commit that referenced this issue May 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants