ocaml-semver / ocaml-api-watch

Libraries and tools to keep watch on you OCaml lib's API changes
ISC License
21 stars 15 forks source link

Improved Diff type to use for text diff #59

Closed Siddhi-agg closed 4 months ago

Siddhi-agg commented 4 months ago

Fixes #57.

This PR aims to make the following changes:

There is still work left to be done for #57, which will allow us to print a text diff from the diff type returned by diff_interface function.

Siddhi-agg commented 4 months ago

Ideally, we would like to expand the type only if it's necessary for the diff to be readable but that's a bit hard to do with our current representation and probably not worth the effort as I expect having a more precise diff represenation will make that trivial.

Yeah, working on a better representation for diffs seems the better way to go, rather than trying to determine if expanding the type is needed for a more readable diff.

Also, I think we can go for resolving type expressions only for the Modified types. I thought resolving it also in case of Added and Removed types would be consistent and thus easy for the users to understand. But what you say makes sense, so we can go ahead with that.

How about adding a test that checks that the value_description type is properly expanded, that would clear things.

Yeah, that's a good idea! I will work on that.