This is an archive of the discontinued LLVM Phabricator instance.

[llvm] [bindings/OCaml] Remove unused dep on ounit2
ClosedPublic

Authored by mgorny on Feb 15 2022, 12:25 PM.

Details

Summary

Remove the dependency on ounit2 and the relevant lit code. It seems
that ounit2 is not used at all and all OCaml binding tests pass without
it installed.

Thanks for Josh Berdine for noticing.

Diff Detail

Event Timeline

mgorny requested review of this revision.Feb 15 2022, 12:25 PM
mgorny created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2022, 12:25 PM
jberdine accepted this revision.Feb 16 2022, 7:03 AM
jberdine added a subscriber: arbipher.

Looks good, thanks!

Note that it was @arbipher who first noticed that ounit is no longer used.

This revision is now accepted and ready to land.Feb 16 2022, 7:03 AM
arbipher accepted this revision.Feb 16 2022, 8:07 AM

It looks good to me.

My 5 cent (for ocaml) is to keep it and it would be easier to really add some useful ounit tests, however, this PR also has shown what needs to do to revert it back.

Note that it was @arbipher who first noticed that ounit is no longer used.

Ok, thanks for the correction and sorry for the mistake.

My 5 cent (for ocaml) is to keep it and it would be easier to really add some useful ounit tests, however, this PR also has shown what needs to do to revert it back.

Yeah, we can easily revert this if we ever introduce ounit-based tests.

This revision was landed with ongoing or failed builds.Feb 16 2022, 10:30 AM
This revision was automatically updated to reflect the committed changes.

Do you think this should be backported to 14.x? On one hand, it isn't exactly urgent, but it kinda sucks to require this unused dep for another release.