Skip to content

haddock-project: add CommonSetupFlags - #10393

Merged
mergify[bot] merged 1 commit into
haskell:masterfrom
MercuryTechnologies:wiggles/haddock-project-parse-common-flags
Oct 11, 2024
Merged

haddock-project: add CommonSetupFlags#10393
mergify[bot] merged 1 commit into
haskell:masterfrom
MercuryTechnologies:wiggles/haddock-project-parse-common-flags

Conversation

@9999years

@9999years 9999years commented Sep 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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
geekosaur
geekosaur approved these changes Sep 27, 2024
View reviewed changes

@geekosaur geekosaur left a comment

Copy link
Copy Markdown
Collaborator

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 mentioned this pull request Sep 28, 2024
Merged
5 tasks
@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
Closed
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
mpickering reviewed Oct 2, 2024
View reviewed changes
Comment thread cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs Outdated
mpickering
mpickering reviewed Oct 2, 2024
View reviewed changes
Comment thread Cabal/src/Distribution/Simple/Setup/Haddock.hs
@mpickering

mpickering commented Oct 2, 2024

Copy link
Copy Markdown
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

9999years commented Oct 2, 2024

Copy link
Copy Markdown
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

Kleidukos commented Oct 9, 2024

Copy link
Copy Markdown
Member

@coot ping

@ulysses4ever

ulysses4ever commented Oct 10, 2024

Copy link
Copy Markdown
Collaborator

Note: API change, can't be backported.

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

@ulysses4ever

ulysses4ever commented Oct 10, 2024

Copy link
Copy Markdown
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

gbaz commented Oct 10, 2024

Copy link
Copy Markdown
Collaborator

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
coot
coot approved these changes Oct 11, 2024
View reviewed changes

@coot coot left a comment

Copy link
Copy Markdown
Collaborator

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

9999years commented Oct 11, 2024

Copy link
Copy Markdown
Collaborator Author

Rebased for mergify permissions

@geekosaur

geekosaur commented Oct 11, 2024

Copy link
Copy Markdown
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
@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

Reviewers

@mpickering mpickering mpickering left review comments
@coot coot coot approved these changes
@gbaz gbaz gbaz approved these changes
@geekosaur geekosaur geekosaur approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@9999years @mpickering @Kleidukos @ulysses4ever @gbaz @geekosaur @coot