Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Differential D119884

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

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

Details

Reviewers

jberdine
vaivaswatha
arbipher

Commits

rG2c5c243bb515: [llvm] [bindings/OCaml] Remove unused dep on ounit2

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

Repository: rG LLVM Github Monorepo

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

Harbormaster completed remote builds in B149797: Diff 409009.Feb 15 2022, 3:10 PM

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

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.

In D119884#3326249, @jberdine wrote:

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

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

In D119884#3326515, @arbipher wrote:

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

Closed by commit rG2c5c243bb515: [llvm] [bindings/OCaml] Remove unused dep on ounit2 (authored by mgorny). · Explain Why

This revision was automatically updated to reflect the committed changes.

mgorny added a commit: rG2c5c243bb515: [llvm] [bindings/OCaml] Remove unused dep on ounit2.

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.