feat: add lte method for connectors mapping language #7722
Conversation
|
@andrewmcgivery, please consider creating a changeset entry in |
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 1 changed, 0 removedBuild ID: 083c564afe6091bbd6e5e382 URL: https://www.apollographql.com/docs/deploy-preview/083c564afe6091bbd6e5e382 |
| match (data, &value) { | ||
| // Number comparisons | ||
| (JSON::Number(left), JSON::Number(right)) => { | ||
| left.as_f64().unwrap_or(0.0) <= right.as_f64().unwrap_or(0.0) |
There was a problem hiding this comment.
I don't think substituting 0.0 here is a good idea, especially since it returns true if both unwrappings fail. Let's return false with an error if either conversion fails?
| | (ShapeCase::String(_), ShapeCase::String(_)) => { | ||
| Shape::bool(method_name.shape_location(source_id)) | ||
| } | ||
| _ => Shape::error( |
There was a problem hiding this comment.
This makes it an error if the input shape is ShapeCase::Name, which could become/represent a number at runtime. Instead of lumping that case into the _ => Shape::error branch, let's make it explicit.
| ); | ||
|
|
||
| match (arg_shape.case(), input_shape.case()) { | ||
| (ShapeCase::Unknown, ShapeCase::Unknown) |
There was a problem hiding this comment.
I don't think we should require both shapes to be Unknown here. As long as one of the shapes is among the allowed cases below, the other shape can be Unknown. Of course both Unknown is also allowed, just not required.
There was a problem hiding this comment.
Possibly similar logic: #7717 (comment)
Add lte method for connectors mapping language)
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
A lot of (if not most) features benefit from built-in observability and
debug-level logs. Please read this guidance on metrics best-practices. ↩Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩