Skip to content

Use ProjectFlags to define CleanCmd #9356

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 25, 2023
Merged

Use ProjectFlags to define CleanCmd #9356

merged 4 commits into from
Oct 25, 2023

Conversation

Image for: Conversation
Copy link
Contributor

shapr commented Oct 20, 2023 •
edited
Loading

Reduce duplication of project flag handling, fixes #6816

The nearly identical PR for #7439 was used as a guide for this PR.
Use ProjectFlags to define CleanCmd
4771b9c 
The nearly identical PR for haskell#7439 was used as a guide for this PR.
The point of this PR is to reduce the duplication of project flag
handling.

Co-authored-by: Jean-Paul Calderone <exarkun@twistedmatrix.com>
geekosaur approved these changes Oct 20, 2023
View reviewed changes
Copy link
Collaborator

geekosaur left a comment

Choose a reason for hiding this comment

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

I get the impression NamedFieldPuns is not well liked here, but otherwise it's okay.
Copy link
Collaborator

Thanks for the contribution!

I'm fine with field puns but it's strange to see it turned on together with wildcards -- their purpose largely intersects. Also, where are puns used? I couldn't spot it when skimming the text of the patch.
Copy link
Collaborator

exarkun commented Oct 20, 2023

Thanks for the contribution!

I'm fine with field puns but it's strange to see it turned on together with wildcards -- their purpose largely intersects. Also, where are puns used? I couldn't spot it when skimming the text of the patch.

Just in the definition of cleanAction, unpacking flagProjectName. Happy to remove the use of this extension if that's preferred, we just copied the style from CmdOutdated.hs
Copy link
Collaborator

Oh, I see now. Yeah, for consistency, it'd make sense to replace it with wildcards, I think. But only because of that. I personally prefer puns to wildcards.
ulysses4ever approved these changes Oct 20, 2023
View reviewed changes
Copy link
Collaborator

ulysses4ever 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
Collaborator

exarkun commented Oct 23, 2023

Thanks for the reviews!

Per the contributor guide, now labeling for merge.
exarkun added the squash+merge me Tell Mergify Bot to squash-merge label Oct 23, 2023
Copy link
Collaborator

@exarkun thank you! ❤️
mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Oct 25, 2023
mergify bot merged commit 0bab7cb into haskell:master Oct 25, 2023
shapr deleted the 6816.cmdclean-project-flags branch November 8, 2023 17:35
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 squash+merge me Tell Mergify Bot to squash-merge
4 participants