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

Refactor extraction and caching of CabalInfo #1034

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

brandonchinn178
Copy link
Collaborator

Broken off from #1033

This change has the following improvements:

  1. Force user to handle missing stanza information (e.g. dependencies) when the cabal file does not mention a source file
  2. Make CabalInfo return all info for the cabal file, without reference to the source file
    • Simplifies parseCabalInfo as just "parse cabal file", not "parse cabal file + extract info relevant to given source file"
    • IMO this is clearer, given the names of the types and functions.

@mrkkrp mrkkrp force-pushed the cabal-not-mention branch from dde7319 to 77a5cd9 Compare June 2, 2023 13:36
@mrkkrp
Copy link
Member

mrkkrp commented Jun 2, 2023

I'm happy with the first commit, which I adjusted a bit to apply the new helper to both caching of cabal files and .ormolu files, however I don't think we can easily merge something that changes the signature of refineConfig because it changes the API, which we are supposed to keep backwards compatible now as per c81320d.

@mrkkrp mrkkrp force-pushed the cabal-not-mention branch from 77a5cd9 to 32bc6f4 Compare June 2, 2023 13:50
@brandonchinn178
Copy link
Collaborator Author

Ah sure, I could make the changes backwards compatible. Give me a second

@brandonchinn178 brandonchinn178 force-pushed the cabal-not-mention branch 2 times, most recently from da1c023 to c0bf761 Compare June 2, 2023 16:05
@brandonchinn178
Copy link
Collaborator Author

brandonchinn178 commented Jun 2, 2023

@mrkkrp how's that?

Also, there's a weeder failure. Maybe weeder.dhall should be updated to include the Ormolu module as a root?

@mrkkrp
Copy link
Member

mrkkrp commented Jun 6, 2023

Eh, I like splitting the code in two branches (old and new) even less. Let's apply d91bbd0 and keep the rest on the shelf for a while until we decide to release 0.8.0.0 at which point we can make a clean break.

@mrkkrp mrkkrp changed the title Ensure "source file not mentioned in Cabal file" has same behavior as "no Cabal file found" Refactor extraction and caching of CabalInfo Jun 6, 2023
@brandonchinn178
Copy link
Collaborator Author

Sure up to you

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.

2 participants