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

Define BuildTarget in new BuildTarget.Types module #9472

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sheaf
Copy link
Collaborator

@sheaf sheaf commented Nov 24, 2023

This change means that we can refer to BuildTarget in Cabal error constructors. (Attempting to do so before this change would cause cyclic imports.)

@sheaf sheaf marked this pull request as draft November 24, 2023 13:46
This change means that we can refer to BuildTarget in Cabal error
constructors. (Attempting to do so before this change would cause
cyclic imports.)
@andreabedini
Copy link
Collaborator

Sub component targets had never been implemented and that is mostly dead-code. I think we should consider removing this feature, including from cabal-install. I had PR that did that but it got bitrotten by the advent of fourmolu.

@sheaf
Copy link
Collaborator Author

sheaf commented Nov 29, 2023

Sub component targets had never been implemented and that is mostly dead-code. I think we should consider removing this feature, including from cabal-install. I had PR that did that but it got bitrotten by the advent of fourmolu.

Ah yes, I see, that was #8966.

I'm not sure we will end up needing the changes in this PR in the end, so I'm happy to park it for the time being.

@sheaf
Copy link
Collaborator Author

sheaf commented Mar 5, 2024

I think this refactor still makes sense, but nothing important depends on it right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants