Conversation
| new_inner, | ||
| )?)) | ||
| } | ||
| (BareRef | InlineType | MultiElement, BareRef | InlineType | MultiElement) |
There was a problem hiding this comment.
Thanks for doing this. My understanding is that the breakage/test case falls into this category where we end up returning None -- is that accurate?
There was a problem hiding this comment.
That's right. It's an allof ref pointing to an allof ref--a bit unusual
| fn compare_schema_type_all_of( | ||
| &mut self, | ||
| comparison: SchemaComparison, | ||
| dry_run: bool, | ||
| old_all_of: Contextual<'_, &Vec<ReferenceOr<Schema>>>, | ||
| new_all_of: Contextual<'_, &Vec<ReferenceOr<Schema>>>, | ||
| ) -> anyhow::Result<bool> { |
There was a problem hiding this comment.
not totally important but I'm wondering if we can make allOf, oneOf and anyOf (which I believe is inlined into compare_schema_kind) look as similar to each other as possible. It feels like they should generally be symmetric, at least in the length = 1 case.
| if old_any_of != new_any_of { | ||
| self.schema_push_change( | ||
| dry_run, | ||
| "unhandled, 'anyOf' schema", | ||
| &old_schema_kind, | ||
| &new_schema_kind, | ||
| comparison, | ||
| ChangeClass::Unhandled, | ||
| ChangeDetails::UnknownDifference, | ||
| ) | ||
| } else { | ||
| Ok(true) | ||
| } |
There was a problem hiding this comment.
paging the context back in (was trying to figure out how I screwed up, haha)... I think this is theoretically buggy in the same way, though single-element anyOf is not idiomatic so we probably won't hit it in practice.
(let me know if these comments are unhelpful)
There was a problem hiding this comment.
There was a problem hiding this comment.
Yeah I realized that anyOf wasn't handled and I figured if it came up we might want to actually know about it. I could see it either way
2 participants
It looks like #10 broke an untested case that arose in oxidecomputer/omicron#10037