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

WIP/Search upwards for package directory #2811

Closed

Conversation

BardurArantsson
Copy link
Collaborator

FOR REVIEW ONLY

@BardurArantsson
Copy link
Collaborator Author

High-level overview of what I've done here:

  • Added a new SandboxMetadata that contains both the sandbox
    directory and the config file path.
  • I've changed the UseSandbox construct to take such a
    SandboxMetadata value -- this simplifies the lookup logic
    drastically since it can be unified in a single function which
    handles both the lookup of the .cabal file and the
    configuration file. There are some cases error cases that would
    be very hard to detect if we didn't combine the logic this.
    (See getSandboxMetadata for the search/lookup logic.)

Observation: The config flag handling seems rather messy -- I
find it very hard to judge which configuration is actually being
used.

@BardurArantsson
Copy link
Collaborator Author

See also comments on the #2810 issue regarding implementation

@BardurArantsson
Copy link
Collaborator Author

Oh, yeah, I haven't spent any time actually testing this yet -- I just want to get feedback on the approach before I spend any time on adding/updating tests. I expect a feature this complex will require some PackageTests at the very least.

@hvr
Copy link
Member

hvr commented Sep 4, 2015

Thanks for doing this.

Could you maybe outline the algorithm you use (or plan to use) for locating the various config-files?

@BardurArantsson
Copy link
Collaborator Author

I'm closing this for now. I think it'll need a bit of reworking and most of all TESTS. Hopefully, #2797 will be enough :).

@BardurArantsson
Copy link
Collaborator Author

Meh, meant #2868. (Hard to follow all the indirections!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants