-
Notifications
You must be signed in to change notification settings - Fork 19
feat: allow title to have only one character #894
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
base: main
Are you sure you want to change the base?
feat: allow title to have only one character #894
Conversation
|
ping @canonical/starcraft-reviewers - related to github.com/canonical/rockcraft/pull/957 |
|
I believe the reason we made the title require a minimum length of 2 is because the store rejects items with a title length of 1. When looking for that, I couldn't find that declaration, but I did find that the store will reject rocks with a single-character name (https://git.launchpad.net/review-tools/tree/reviewtools/schemas/rock.json), which perhaps means we should rethink canonical/rockcraft#957? (I think at minimum a single-character name is a reason for a warning that the store will reject the rock.) |
Adding to this, the snap store does have a declaration for the title and seems to allow single character titles, but not names. (https://git.launchpad.net/review-tools/tree/reviewtools/schemas/snap.json)
I believe the main issue is Rockcraft setting the title equal to the name when title is not set. I believe that At this point trying to to set We could think about adding a Lmk your thoughts |
Well this is why I think it's worth revisiting canonical/rockcraft#957 - even if the user explicitly sets the title, a single-character name is something that'll be rejected by the store. So Rockcraft should be requiring a name of at least 2 characters, in line with canonical/rockcraft#943. In fact, it might be a bug in snapcraft that it allows single-character names since the store will always reject them
The title field isn't required in any craft, though it is recommended in most.
Correct, but that's part of why I'm being such a stick-in-the-mud about this single-byte change. :-) The values here are supposed to represent the loosest definition of what would be appropriate for a future craft, but existing crafts can diverge from this standard in both direction if they must. I'm not opposed to loosening the requirements for titles in principle, but I need to determine why it was set this way to begin with, and unfortunately the guy who put that constraint into this code (me) didn't leave me any clues. On the flip side, making the name length in Rockcraft match what the review tools require (length of 2) before the next release, wouldn't be a breaking change (it would still be looser than the current requirement of 3). So that's probably a better solution in the short term while we carefully consider this change across a variety of crafts. I made a PR to that effect here: canonical/rockcraft#972 |
This is my solution to the impasse in canonical/craft-application#894 ROCKS-2421 Signed-off-by: Alex Lowe <alex.lowe@canonical.com>
make lint && make test?docs/reference/changelog.rst)?This PR decreases the
min_lengthof the project title to 1.Rationale
Some of the craft-applications (rockcraft, snapcraft, etc) set the project title equal to the name when the former is not set. However, the
nameallows single character values while being more restrictive (regex) meaning that setting a (valid) name with a single character would fail the validation oftitle.Proposed solution: Make the
titleequal or less restrictive thannameby reducing themin_lengthto1.