Skip to content
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

create branch name validation message #3374

Merged
merged 2 commits into from
May 22, 2022
Merged

create branch name validation message #3374

merged 2 commits into from
May 22, 2022

Conversation

eden-ohana
Copy link
Contributor

fixes #2894

argument branch: branch id: invalid value: validation error can contain only letters, numbers, underscores, and dashes (not at the beginning)

@eden-ohana eden-ohana added the include-changelog PR description should be included in next release changelog label May 17, 2022
@eden-ohana eden-ohana requested a review from itaiad200 May 17, 2022 09:44
@@ -23,7 +23,7 @@ var (
ErrNoMergeBase = errors.New("no merge base")
ErrInvalidRef = fmt.Errorf("ref: %w", ErrInvalidValue)
ErrInvalidCommitID = fmt.Errorf("commit id: %w", ErrInvalidValue)
ErrInvalidBranchID = fmt.Errorf("branch id: %w", ErrInvalidValue)
ErrInvalidBranchID = fmt.Errorf("branch id: %w can contain only letters, numbers, underscores, and dashes (not at the beginning)", ErrInvalidValue)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will have the form

branch id: invalid value: can contain only letters, numbers, underscores, and dashes (not at the beginning)

which looks somewhat different from ErrInvalidRef and friends. Consider

Suggested change
ErrInvalidBranchID = fmt.Errorf("branch id: %w can contain only letters, numbers, underscores, and dashes (not at the beginning)", ErrInvalidValue)
ErrInvalidBranchID = fmt.Errorf("branch id must consist of letters, digits, underscores and dashes, and cannot start with a dash: %w", ErrInvalidValue)

However I would also like to question whether we need this validation almost anywhere except during creation. Specifically, if I try to create a branch named '-not=valid--` then it should fail. But anywhere else I only fetch a branch, and I think I should be just as happy to be told "branch not found".

@eden-ohana
Copy link
Contributor Author

message: "argument branch: branch id: invalid value: validation error: branch id must consist of letters, digits, underscores and dashes, and cannot start with a dash"

returns only on create branch validation

@eden-ohana eden-ohana merged commit f14b220 into master May 22, 2022
@eden-ohana eden-ohana deleted the 2894/fix branch May 22, 2022 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include-changelog PR description should be included in next release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation and returned error should declare naming conventions
3 participants