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

When contributor name contains a minus sign then allcontributors loses the part after the minus sign #354

Closed
aaltat opened this issue Apr 10, 2021 · 9 comments · Fixed by all-contributors/all-contributors#525

Comments

@aaltat
Copy link

aaltat commented Apr 10, 2021

Describe the bug
If contributor name contains a minus sign, like foo-bar, then allcontributors loses the part after the minus sign. Example foo-bar comes as foo-

To Reproduce

  1. When I example says: @all-contributors please add vincenzo-gasparo for code
  2. Then Could not find the user vincenzo- on github.

Expected behavior
Did expect user to be assed correctly.

@Berkmann18
Copy link
Member

That's odd because all-contributors/all-contributors#413 was supposedly (recently) fixed.
What version of the CLI are you using?

@Berkmann18 Berkmann18 transferred this issue from all-contributors/all-contributors Apr 13, 2021
@aaltat
Copy link
Author

aaltat commented Apr 14, 2021

Interesting, our repository is using AllContributors bot.

image

@Berkmann18 Berkmann18 transferred this issue from all-contributors/cli Apr 14, 2021
@Berkmann18
Copy link
Member

Have you tried doing that with the CLI by any chance? If so, did you also got an error?

I've dug a bit deeper and this might be a regression as opposed to a bug (given that it worked for users who used the bot, although that was before it was rewritten).

@aaltat
Copy link
Author

aaltat commented Apr 16, 2021

I tried from command line and from command line user name was detected correctly.

@aaltat
Copy link
Author

aaltat commented Apr 16, 2021

Found the PR from our repository where the problem occurs: MarketSquare/robotframework-browser#814 (comment)

@mtfoley
Copy link

mtfoley commented May 2, 2021

FWIW, it looks like the parsing would work if using "@olga-zm". I added a couple of tests just to see what it would do.
Screen Shot 2021-05-02 at 8 47 30 AM
Screen Shot 2021-05-02 at 8 49 17 AM

@Berkmann18
Copy link
Member

@mtfoley I see, feel free to submit a PR with your findings.

@mtfoley
Copy link

mtfoley commented May 8, 2021

@Berkmann18 I actually think you should close the issue and @aaltat should try using the bot with username mentions in the future. The changes introduced by the merged PR #434 for all-contributors/all-contributors#413 was to update the docs to show preceding contributor username with @. I think the docs ought to go one step further and explicitly state that the bot will match the username best if preceded with @ symbol. I'll submit a PR over there.

mtfoley added a commit to mtfoley/all-contributors that referenced this issue May 8, 2021
What: Added a more explicit note. @-mentioning when using the bot. Should Fix [all-contributors/app#354](all-contributors/app#354)

Why: To be a little more explicit that bot will work best with @-mentions.

How: Added clarifying note below the note about natural language processing.

Checklist:

[x] Documentation
[x] Ready to be merged
[ ] Added myself to contributors table.
@mtfoley
Copy link

mtfoley commented May 8, 2021

See PR #525

Berkmann18 pushed a commit to all-contributors/all-contributors that referenced this issue May 8, 2021
What: Added a more explicit note. @-mentioning when using the bot. Should Fix [all-contributors/app#354](all-contributors/app#354)

Why: To be a little more explicit that bot will work best with @-mentions.

How: Added clarifying note below the note about natural language processing.

Checklist:

[x] Documentation
[x] Ready to be merged
[ ] Added myself to contributors table.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants