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

src_ext: Remove unused/untested Makefile targets #5494

Merged

Conversation

kit-ty-kate
Copy link
Member

I don't think anyone use that, and we're not testing it either.
Also it complicates vendoring of dependencies so I feel like any simplification in there is always welcome.

@dra27
Copy link
Member

dra27 commented Mar 27, 2023

Slightly in passing - I'm completely for removing this, but preferably after 2.2 (i.e. after there's a released Windows opam which does away with the use case for it). It is useful (and used, certainly by me!) for developing opam on Windows, because the build is a lot faster than the vendored route when working on opam itself. It's also in use in the base image builder at the moment. It is also tested - see dra27@ee6e3a5 which demonstrates the breakage this commit presently causes in the Cold job (the Cold job in the PR is passing because #4776 means that the build is quietly downloading the dependencies which haven't been installed).

@kit-ty-kate kit-ty-kate added this to the 2.3 milestone Mar 27, 2023
@kit-ty-kate kit-ty-kate removed this from the 2.3 milestone Mar 4, 2024
@kit-ty-kate kit-ty-kate force-pushed the rm-unused-vendoring-primitives branch 2 times, most recently from 08cfd31 to a527326 Compare May 20, 2024 11:20
@dra27 dra27 self-requested a review May 20, 2024 12:17
@kit-ty-kate kit-ty-kate requested a review from rjbou May 21, 2024 22:31
@kit-ty-kate kit-ty-kate force-pushed the rm-unused-vendoring-primitives branch from a527326 to 2a57626 Compare May 27, 2024 20:59
Copy link
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RIP, with fond memories of the workshop in Princeton where I was testing most of this 🙂

@kit-ty-kate kit-ty-kate merged commit 945a98a into ocaml:master Jun 4, 2024
29 checks passed
@kit-ty-kate kit-ty-kate deleted the rm-unused-vendoring-primitives branch June 4, 2024 12:25
@kit-ty-kate kit-ty-kate added this to the 2.2.0~beta3 milestone Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants