Skip to content

fix: improve invalid slice name error - #240

Merged
niemeyer merged 1 commit into
canonical:mainfrom
gajeshbhat:fix-slice-name-error-message-158
Aug 29, 2025
Merged

niemeyer merged 1 commit into
canonical:mainfrom
gajeshbhat:fix-slice-name-error-message-158

Conversation

@gajeshbhat

Copy link
Copy Markdown
Contributor

This PR improves the error message for invalid slice names to provide clearer guidance to users about the validation requirements, as requested in #158.

Changes

  • Enhanced the error message in internal/setup/yaml.go to clarify that slice names must be at least 3 characters long
  • Added detailed explanation of all slice name validation requirements in the error message
  • Added unit test to verify that the improved error message is displayed correctly

Before

error: invalid slice name cc in slices/base-distroless.yaml

After

error: invalid slice name cc in slices/base-distroless.yaml (slice names must be at least 3 characters long, start with a lowercase letter, and contain only lowercase letters, digits, and hyphens)

Testing

  • All existing tests pass
  • Added a new unit test case for the improved error message
  • Manually verified that the error message appears correctly for various invalid slice names

Fixes #158

@github-actions

github-actions Bot commented Aug 16, 2025 •

Copy link
Copy Markdown
Command Mean [s] Min [s] Max [s] Relative
BASE 8.829 ± 0.049 8.757 8.890 1.00
HEAD 8.833 ± 0.025 8.802 8.868 1.00 ± 0.01

@letFunny
letFunny self-requested a review August 21, 2025 07:58

@letFunny letFunny left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thank you for taking the time to work on this. I agree that we should improve the error messages to some extent to improve the UX. At the same time, writing prose to describe the regular expression might not be ideal, it will always be less precise and too verbose.

Let's see if we can create a message that is short and simple.

Comment thread internal/setup/yaml.go Resolved
@gajeshbhat
gajeshbhat force-pushed the fix-slice-name-error-message-158 branch from ac9f29a to 62e6572 Compare August 23, 2025 08:54
@gajeshbhat
gajeshbhat requested a review from letFunny August 23, 2025 08:59
- Add improved error message
- Add unit test to verify the improved error message

Fixes #158

@letFunny letFunny left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into improving the UX @gajeshbhat! LGTM

Comment thread internal/setup/setup_test.go Resolved
@letFunny letFunny added Simple Nice for a quick look on a minute or two Polish Refactorings, etc labels Aug 28, 2025
niemeyer

This comment was marked as outdated.

@gajeshbhat gajeshbhat changed the title fix: improve slice name validation error message clarity fix: improve invalid slice name error Aug 28, 2025
@gajeshbhat

gajeshbhat commented Aug 28, 2025 •

Copy link
Copy Markdown
Contributor Author

Looks good, thanks.

Thank you for the review @niemeyer . Appreciate you taking the time.

Comment thread internal/setup/setup_test.go Resolved
@niemeyer
niemeyer merged commit 680d53c into canonical:main Aug 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Polish Refactorings, etc Simple Nice for a quick look on a minute or two

Projects

None yet

Development

Successfully merging this pull request may close these issues.

invalid slice name error should clarify min valid length is 3

3 participants