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

Enrich ModelDiff with a data structure describing shape differences #2158

Open
miguel-vila opened this issue Feb 22, 2024 · 2 comments
Open
Labels
feature-request A feature should be added or improved.

Comments

@miguel-vila
Copy link
Contributor

miguel-vila commented Feb 22, 2024

Hey, in order to debug something in our logic, we had to kind of replicate the logic in Shape.equals (and in the subclasses). We are using ModelDiff, which provides with some basic diff information, but we needed to debug things a bit more. What we ended up doing was partially replicating Shape.equals but returning a data structure that would tell us what was the reason why the shapes are not considered equal.

For example, if the reason shape1.equals(shape2) returns false is a trait addition and a trait modification, then we would return (in Scala) something like List(Diffs.TraitsChanged(removed=List.empty, added = List(Trait(shapeId= ..., value = ...)), modified = List(TraitModified(valueBefore = ..., valueAfter = ...)))) .

We were wondering if Smithy could have this logic instead, as we don’t want to risk replicating and diverging from this. I think this could be a new field in ChangedShape describing the difference. Let us know what you think

@JordonPhillips JordonPhillips added the feature-request A feature should be added or improved. label Feb 27, 2024
@JordonPhillips
Copy link
Contributor

JordonPhillips commented Jun 19, 2024

What information are you not getting from ModelDiff? Or what other issues are you having using it?

@miguel-vila
Copy link
Contributor Author

we use ModelDiff and ChangedShape<Shape>. The issue with ChangedShape is that it doesn't tell use what specifically changed, only the old and new shape. We figure out the difference on our own (e.g. it was a type change or a member addition, etc...), and that's where we replicate Shape.equals. If I'm not mistaken, ModelDiff creates a ChangedShape for each shape if !oldShape.equals(newShape).

We have some logic that is based on whether there's a difference or not, and sometimes it's not enough to know that a particular shape changed. In addition, we want to see what difference was detected, to debug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

2 participants