-
Notifications
You must be signed in to change notification settings - Fork 9
builtins: numbers.range and range_step #10
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
|
Compliance suite results: |
d6e14df to
6da561a
Compare
336ca51 to
8354388
Compare
srenatus
left a comment
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.
Limited swift knowledge
Sources/Rego/Builtins/Numbers.swift
Outdated
| // Golang error is different: step must be a positive number above zero. | ||
| // But positive number is by definition above zero... | ||
| // so using a different error message. |
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.
https://github.com/open-policy-agent/opa/pull/7882/files This has happened since then 😉
| BuiltinTests.generateFailureTests( | ||
| builtinName: "numbers.range", sampleArgs: [1, 1], argIndex: 1, argName: "b", | ||
| allowedArgTypes: ["number"], | ||
| generateNumberOfArgsTest: false), |
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.
💭 So I guess these cover the number of args in some way, and since we're doing this with the first arg (above, and in the argName="a" case below), that's sufficient. Sounds fair to me.
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.
Yes, there are similar calls below each arg....
Implement numbers.range and numbers.range_step builtins maintaining compatibility with Go version. Resolves open-policy-agent#6 Signed-off-by: Dmitry Frenkel <[email protected]>
636fe4d to
2363ff5
Compare
srenatus
left a comment
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.
LGTM
Implement numbers.range and numbers.range_step builtins maintaining compatibility with Go version.
Resolves #6