-
Notifications
You must be signed in to change notification settings - Fork 150
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
fix: support usernames with multiple dashes #463
fix: support usernames with multiple dashes #463
Conversation
@brandonroberts is attempting to deploy a commit to the All Contributors Team on Vercel. A member of the Team first needs to authorize it. |
8a84507
to
bcf93a9
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #463 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 451 451
Branches 53 53
=========================================
Hits 451 451
☔ View full report in Codecov by Sentry. |
9ec1b4b
to
1f12645
Compare
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
"rishi-raj-jain": ['doc'], | ||
}, | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test is not failing. Looks like the existing code is working, even without your change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here is a test, seems like the username is parsed okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that's where I traced it down to parsing the username. It shows a valid username not being parsed here analogjs/analog#362 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm okay I'll do some more testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay I was able to reproduce the problem: gr2m/sandbox#252 (comment).
As I said the test is not failing the implementation of your fix though. Could you please have a look? I'll meanwhile deploy your changes to staging and test that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confirmed your change is working: gr2m/sandbox#252 (comment). So all it's left is to make your test fail before the fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, thanks
@all-contributors add @brandonroberts for code and test |
I've put up a pull request to add @brandonroberts! 🎉 |
🎉 This PR is included in version 1.19.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What:
Fixes a bug where a username with multiple dashes is not formatted correctly and the username is not found
Why:
analogjs/analog#362 (comment)
How:
Replaces all occurrences of a dash in the username. Removes all occurrences of the replacement characters
Checklist:
Bot Usage