Skip to content

Set exe as default component for cabal run and cabal list-bin #7408

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

Merged
merged 3 commits into from
Dec 1, 2021

Conversation

dminuoso
Copy link

@dminuoso dminuoso commented May 25, 2021

Resolves #7403

The modification was tested against https://github.com/dminuoso/cabal-repro and the added integration test.

Documentation will be updated tomorrow.


Please include the following checklist in your PR:

Please also shortly describe how you tested your change. Bonus points for added tests!

@Mikolaj
Copy link
Member

Mikolaj commented May 26, 2021

@dminuoso, @emilypi, @fgaz: who could review in addition to me? Better sooner rather than later to avoid wasting additional author's effort in case I have mixed things up and the spec (in the original issue) makes no sense, e.g., there was some important purpose for the old behaviour.

Copy link
Member

@fgaz fgaz left a comment

Choose a reason for hiding this comment

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

I did not test it, but the code looks good! I like how the names are more consistent now

@dminuoso dminuoso force-pushed the add_default_component_for_packages branch 2 times, most recently from 219fffc to cc5bd6f Compare May 27, 2021 17:33
@Mikolaj
Copy link
Member

Mikolaj commented May 31, 2021

@dminuoso: the topic starts with [WIP] --- is it ready for final review and merge in your opinion? Do you need any extra help?

@dminuoso
Copy link
Author

dminuoso commented Jun 1, 2021

@Mikolaj Hi, I was waiting for @phadej to weigh in on #7403 (comment)

But we can merge this already if you like. If phadej agrees to align list-bin with run wrt to implicit packages, I can make a separate PR.

@dminuoso dminuoso changed the title [WIP] Set exe as default component for cabal run and cabal list-bin Set exe as default component for cabal run and cabal list-bin Jun 1, 2021
@Mikolaj
Copy link
Member

Mikolaj commented Jun 1, 2021

@phadej, could you weigh in? :)

@Mikolaj
Copy link
Member

Mikolaj commented Jun 8, 2021

I'm afraid we need to make do with our own weight. What are you opinions? @dminuoso, @fgaz, others?

@fgaz
Copy link
Member

fgaz commented Jun 8, 2021

Well, whatever the choice for list-bin is, we can merge this anyway, no?

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

LGTM.

I guess a changelog entry wouldn't hurt, though one can argue this is neither a surprising nor crucial change to how cabal operates.

@Mikolaj
Copy link
Member

Mikolaj commented Nov 30, 2021

Ugh, CI apparently got stuck and so the PR got stuck. Let me try to move this forward.

@Mikolaj
Copy link
Member

Mikolaj commented Nov 30, 2021

@Mergifyio rebase

@jneira jneira force-pushed the add_default_component_for_packages branch from cc5bd6f to 2a066b8 Compare November 30, 2021 19:33
@mergify
Copy link
Contributor

mergify bot commented Nov 30, 2021

rebase

✅ Branch has been successfully rebased

@Mikolaj
Copy link
Member

Mikolaj commented Nov 30, 2021

Phew, it seems, this didn't bit-rot. I'm letting mergify merge. Last moment to object.

@dminuoso, @fgaz: thank you!

@Mikolaj Mikolaj added the merge me Tell Mergify Bot to merge label Nov 30, 2021
@jneira jneira force-pushed the add_default_component_for_packages branch from 2a066b8 to 3d3986f Compare November 30, 2021 21:57
@mergify mergify bot merged commit 720333c into haskell:master Dec 1, 2021
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.

cabal run/list-bin does not default to exe component for package targets
3 participants