Skip to content

Remove unused dependencies in hls-refactor-plugin #3953

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 2 commits into from
Jan 16, 2024

Conversation

jhrcek
Copy link
Collaborator

@jhrcek jhrcek commented Jan 14, 2024

I'd like to do general cleanup of unused dependencies (stuff reported by -Wunused-packages).

This PR starts by just doing this for one plugin and checking that people agree with the approach to removing unused ghc (direct - I know that most plugins actually do depend on GHC transitively) dependency.

If you find such initiative useful, I can continue with other plugins.

@jhrcek jhrcek requested a review from santiweight as a code owner January 14, 2024 14:28
@soulomoon
Copy link
Collaborator

soulomoon commented Jan 14, 2024

LGTM, I support such changes

@michaelpj
Copy link
Collaborator

This is very useful. I definitely think we could turn on a lot more warnings generally. I think it's probably worth putting -Wunused-packages into the GHC options for a package when you do this - it means hopefully we stay in the good state afterwards!

@jhrcek
Copy link
Collaborator Author

jhrcek commented Jan 15, 2024

I think it's probably worth putting -Wunused-packages into the GHC options for a package when you do this - it means hopefully we stay in the good state afterwards!

Can do. What would be your general preference for doing that (so we have some consistency) across packages.

  1. Adding -Wunused-packages to each cabal target (library, tests) separately? Or
  2. Adding something like
    common common-warnings 
        ghc-options: -Wunused-packages ...

and then importing for each target?

@michaelpj
Copy link
Collaborator

michaelpj commented Jan 15, 2024

Can do. What would be your general preference for doing that (so we have some consistency) across packages.

Our plugins are pretty inconsistent in general, since they were often written by different people. Adding some consistency would be lovely. I personally like common stanzas for stuff like default-language and ghc-options controlling warnings, so it would be great to use those more consistently.

@jhrcek jhrcek force-pushed the jhrcek/remove-unused-dependencies branch from 046b66b to 3290d18 Compare January 16, 2024 05:00
@jhrcek
Copy link
Collaborator Author

jhrcek commented Jan 16, 2024

I'll work on making these more consistent across multiple plugins, but it's too easy to get sidetracked trying to fix too many things at once. So let me try to maintain laser-focus on fixing unused-packages in as many places as possible and we can agree on how to enforce it consistently at later stage.

@jhrcek jhrcek force-pushed the jhrcek/remove-unused-dependencies branch from 3290d18 to cb91bac Compare January 16, 2024 09:41
@michaelpj michaelpj enabled auto-merge (squash) January 16, 2024 11:19
@michaelpj michaelpj merged commit c113a8b into haskell:master Jan 16, 2024
@jhrcek jhrcek deleted the jhrcek/remove-unused-dependencies branch January 19, 2024 04:14
josephsumabat pushed a commit to josephsumabat/haskell-language-server that referenced this pull request Jan 22, 2024
* Remove unused dependencies in hls-refactor-plugin

* Don't use CPP at all
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.

4 participants