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

haddock-project: add CommonSetupFlags #10393

Conversation

9999years
Copy link
Collaborator

@9999years 9999years commented Sep 27, 2024

Split off of #10292. This lets cabal haddock-project use the standard CommonFlags that other commands use to set verbosity and so on.

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • Manual QA notes have been included.
  • Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

@9999years 9999years force-pushed the wiggles/haddock-project-parse-common-flags branch from f2afeea to 89cb6a8 Compare September 27, 2024 23:35
@9999years 9999years changed the title haddock-project: add CommonFlags haddock-project: add CommonSetupFlags Sep 27, 2024
Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

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

Note: API change, can't be backported.

@9999years 9999years force-pushed the wiggles/haddock-project-parse-common-flags branch from 89cb6a8 to 7fa0e5e Compare September 30, 2024 16:55
@9999years 9999years mentioned this pull request Sep 30, 2024
2 tasks
@9999years 9999years force-pushed the wiggles/haddock-project-parse-common-flags branch from 7fa0e5e to 14510a9 Compare September 30, 2024 17:51
@9999years 9999years marked this pull request as ready for review September 30, 2024 18:40
@9999years 9999years added the merge me Tell Mergify Bot to merge label Sep 30, 2024
@mpickering
Copy link
Collaborator

I am not sure this is the right direction of travel -- why are we propagating the design flaw of reusing the options parsers for the ./Setup interface for the cabal-install interface?

@9999years 9999years force-pushed the wiggles/haddock-project-parse-common-flags branch from 14510a9 to 5730083 Compare October 2, 2024 17:00
@9999years
Copy link
Collaborator Author

I am not sure this is the right direction of travel -- why are we propagating the design flaw of reusing the options parsers for the ./Setup interface for the cabal-install interface?

I can't comment on the design here, but all the other cabal-install commands use this pattern.

@Kleidukos
Copy link
Member

@coot ping

@ulysses4ever
Copy link
Collaborator

Note: API change, can't be backported.

Maybe we should have a label for this case...

@ulysses4ever
Copy link
Collaborator

why are we propagating the design flaw

This PR is not the best time and place to discuss such a big, long-standing issue. I'd prefer this be merged and have the issue discussed separately. But I rarely touch Cabal (as opposed to cabal-install), so maybe I'm wrong.

@gbaz
Copy link
Collaborator

gbaz commented Oct 10, 2024

I agree we should eventually reconsider flag design, but this is not the place. For now let's proceed with the way things are, and it would be good to file an issue for a future refactor.

@mergify mergify bot added merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days ready and waiting Mergify is waiting out the cooldown period labels Oct 10, 2024
Copy link
Collaborator

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM

@9999years 9999years force-pushed the wiggles/haddock-project-parse-common-flags branch from 5730083 to e410770 Compare October 11, 2024 16:00
@9999years
Copy link
Collaborator Author

Rebased for mergify permissions

@geekosaur
Copy link
Collaborator

@9999years , you need to rebase again; see #10445.

@9999years 9999years force-pushed the wiggles/haddock-project-parse-common-flags branch from e410770 to f84087a Compare October 11, 2024 20:04
@mergify mergify bot merged commit 32e3730 into haskell:master Oct 11, 2024
48 checks passed
@9999years 9999years deleted the wiggles/haddock-project-parse-common-flags branch October 11, 2024 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge merge+no rebase ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants