-
Notifications
You must be signed in to change notification settings - Fork 799
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
refactor(allocation): Reimplement the Validate method using "field.ErrorList" #3259
Conversation
Build Failed 😱 Build Id: 80bb142b-9989-4610-a8f3-80e92e296f42 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: a80ce7b5-444b-4692-912d-cb64f8415504 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: e06f6dd4-8ffe-440f-87e9-8f421767e59e To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 553fe32b-9c57-4a87-b196-66fd0fe4ecae To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: e13d7bf4-51c9-4197-98cf-a45e69639b9f To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: aef9c633-2dc5-4f78-ad38-41ca31e02c94 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Message: "Feature CountsAndLists must be enabled if Counters or Lists are specified", | ||
Field: field, | ||
}) | ||
allErrs = append(allErrs, field.Forbidden(fldPath.Child("counters"), "Feature CountsAndLists must be enabled if Counters or Lists are specified")) |
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.
How specific do you want the field path to be? Here there may be the case that "lists" is the field path.
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.
@igooch
Yes, I only processed the counters field, the lists field should have been processed as well, I ignored it
thank you very much
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.
@igooch done
…rorList" Signed-off-by: aimuz <mr.imuz@gmail.com>
Build Succeeded 👏 Build Id: 92eca973-3de0-46c7-a3bf-6b83b0c2ffed The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aimuz, igooch, markmandel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Build Succeeded 👏 Build Id: 530902ae-bd20-4601-988b-969a7f284df7 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
New changes are detected. LGTM label has been removed. |
Build Succeeded 👏 Build Id: 706115f9-bf37-424f-95ca-5f13f43985c7 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 5bcd5397-a20f-47ca-8008-c4d3c3f840bf The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Signed-off-by: aimuz mr.imuz@gmail.com
What type of PR is this?
/kind breaking
What this PR does / Why we need it:
ref: #3239
ref: #3255
Which issue(s) this PR fixes:
Closes #
Special notes for your reviewer: