-
Notifications
You must be signed in to change notification settings - Fork 291
add more restrictive project name check #5862
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
Conversation
|
Nice. For the message, how about: In a future PR we can switch Mainly just avoiding the phrase "will all be invalid". |
|
@aryairani I'm happy to make changes here but I think your suggestion "The following project names will need a quick rename to keep everything running smoothly in upcoming releases" is kinda wordy and confusing to read. Could we perhaps tease out what specifically is wrong with the phrasing "These project names will all be considered invalid in an upcoming UCM release" and then iterate from there? |
|
@mitchellwrosen Good point about it being kind of long. How about
Just trying to match tone with the rest of the UCM, which aims friendlier and less 🤖 "error: invalid data" than most compiler tools. Therefore avoiding the phrase "all will be invalid". |
|
Ok, the point about tone sounds good, I think I still have a small issue with this error message though, which is that this feels unnecessary to disclose to the user (and therefore unnecessarily wordy):
Like, yeah, we screwed up and used two different regexs for project names in two different places and now have to migrate our way out of this situation 😅. How about just starting at the "these project names won't be supported much longer" part? |
|
Yeah, when you put it that way about the regexes...
This is a pretty weak why, but it's better than nothing |
|
CI still seems to be failing, unrelated to this PR |
|
@aryairani Ready |
Overview
Currently UCM allows more project names than Share and the local UI. We've decided to restrict valid project names to slugs similar to GitHub's (except our project names have an optional
@user/prefix):This PR just adds a parser for that syntax, and on startup, reports to the user whether any of their project names don't conform to the new regex: