-
-
Notifications
You must be signed in to change notification settings - Fork 229
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 column githubUsername to users table #4132
Conversation
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 134 (8c0e11) ❌ Edited: 2024-11-14 13:29:59 UTC |
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.
Hi! This makes sense to me, but I'm wondering why we're not just going all the way directly, and identifying all users by GH username always?
@marcelgerber sorry, what do you mean by that? (If your idea is out of scope, can we still merge this? It's needed for a fix with editing users in ETL) |
Sorry, I should've been more clear. Currently, we're still using GH's display name as the identifying feature, e.g. "Mojmir Vinkler". However, with this PR, we are sometimes falling back to GH usernames (e.g. "Marigold"), namely when such a display name is not set, or is not equal to the full name. Would it not make sense to instead always use the GH username (e.g. "Marigold") instead? Looking at the output of "40781454737177829": {
"ID": 40781454737177829,
"LoginName": "Marigold@github",
"DisplayName": "Mojmir Vinkler",
"ProfilePicURL": "https://avatars.githubusercontent.com/u/1550888?v=4",
"Roles": []
}, If you don't want to change that, then feel free to merge it as-is. It's not too hard to change it in the future, too :) |
3fed100
to
3bb3ec3
Compare
@marcelgerber That's a good idea, I was stuck in my "display name" world and didn't see this. Made the 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.
Nice, looks good now!
users.githubUsername
TODO after merging:
make migrate