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

Add -Wunused-packages to common warnings #4053

Merged

Conversation

jhrcek
Copy link
Collaborator

@jhrcek jhrcek commented Feb 5, 2024

No description provided.

@@ -42,18 +42,13 @@ common defaults
common test-defaults
ghc-options: -threaded -rtsopts -with-rtsopts=-N

common common-deps
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Deleting this for now as it's contributing more to unused-packages warnings than it's saving duplication..
Will reintroduce this later where it makes sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

tbh it seems of questionable utility to me anyway

Copy link
Collaborator

Choose a reason for hiding this comment

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

mostly a workaround for not yet having haskell/cabal#9569

@jhrcek jhrcek force-pushed the jhrcek/add-Wunused-packages-to-common-warnings branch from a52b7ba to 012d32e Compare February 6, 2024 06:21
]
-- not supported in ghc92
Copy link
Collaborator Author

@jhrcek jhrcek Feb 6, 2024

Choose a reason for hiding this comment

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

CC @soulomoon I used this value-level CPP-less way to ensure this test only runs with GHC 9.4 and newer.
Did this to be able to get rid of "unused ghc dependency" warning.
While I was at it I fixed all the other warnings in this module.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanx, it looks good

@jhrcek jhrcek force-pushed the jhrcek/add-Wunused-packages-to-common-warnings branch from 737728f to 5224a19 Compare February 7, 2024 13:21
@@ -283,13 +280,6 @@ executable ghcide
if !flag(executable)
buildable: False

if flag(ekg)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ekg dep is only direct dep of ghcide library, not of the executable.

@jhrcek jhrcek force-pushed the jhrcek/add-Wunused-packages-to-common-warnings branch from c9f126d to 47a9dcd Compare February 7, 2024 16:26
@jhrcek jhrcek marked this pull request as ready for review February 7, 2024 20:04
@jhrcek jhrcek requested a review from pepeiborra as a code owner February 7, 2024 20:04
@jhrcek
Copy link
Collaborator Author

jhrcek commented Feb 8, 2024

Ready for review. I'm quite happy about the resulting cleanup 🤗

Copy link
Collaborator

@soulomoon soulomoon left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

Excellent

@michaelpj
Copy link
Collaborator

Looks like a genuine failure, @soulomoon introducing some new warnings for you ;)

@jhrcek
Copy link
Collaborator Author

jhrcek commented Feb 8, 2024

These look more like flaky tests to me. The unused-packages above the failure shouldn't fail the build per-se 🤔
Trying one more rerun..
EDIT: Ah, I see now. I wonder why it fails with stack, but not with cabal 🤔

@jhrcek jhrcek force-pushed the jhrcek/add-Wunused-packages-to-common-warnings branch from 261f00e to 9359811 Compare February 8, 2024 12:06
@soulomoon
Copy link
Collaborator

soulomoon commented Feb 8, 2024

oops, sorry man, I might not have opportunities next time with this pr merged .😂

@jhrcek
Copy link
Collaborator Author

jhrcek commented Feb 8, 2024

No problem, enjoy while you can. Just the usual birth pains of trying to enforce stricter checks 😄

@michaelpj michaelpj enabled auto-merge (squash) February 8, 2024 14:49
@michaelpj michaelpj merged commit 9021c39 into haskell:master Feb 8, 2024
39 checks passed
@jhrcek jhrcek deleted the jhrcek/add-Wunused-packages-to-common-warnings branch February 12, 2024 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants