Detect addition and removal of type declarations

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

Detect addition and removal of type declarations #89

Closed azzsal closed 1 month ago

azzsal commented 1 month ago

This PR aims to close #80. I will follow TDD. I have updated tests in tests/api-watch/test_diff.ml to detect the addition and removal type declarations. The PR is still a work in progress and will updated accordingly.

azzsal commented 1 month ago

I have updated the Diff module to include support for addition and removal of type declarations diffing. The tests fail since the TextDiff module need to be updated to handle type declarations. Before I go ahead and proceed, could you review if my changes to the Diff module are correct ?

Thank you!

azzsal commented 1 month ago

I have updated the Text_diff module and tests to accommodate for type declarations. All tests now pass! I'll appreciate your feedback @NathanReb @panglesd

azzsal commented 1 month ago

Sorry about the messy commits, but I am not familiar with rebase and more used to merge. The cram tests are not passing for some reason, I will try to fix that.

panglesd commented 1 month ago

Indeed, your rebase seems to have gone wrong. There is now more conflicts and more commits!

(I'm personally more used to rebase that's why I suggested it, but you could have done a merge if you prefer!)

Now, my advice would be:

Create a new branch starting on main,
cherry-pick the commits you need from the old branch to this new branch, one by one, resolving conflicts as they come
once everything seems fine, replace the old branch by the new one.

(But maybe there is a better way!)

azzsal commented 1 month ago

This can now be safely closed, since all the work is moved to https://github.com/ocaml-semver/ocaml-api-watch/pull/92. Sorry about the confusion. I'll learn more about git rebasing so this wouldn't happen again.