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

Split up Stack.Constants #1678

Closed
sjakobi opened this issue Jan 19, 2016 · 2 comments
Closed

Split up Stack.Constants #1678

sjakobi opened this issue Jan 19, 2016 · 2 comments

Comments

@sjakobi
Copy link
Member

sjakobi commented Jan 19, 2016

As I found out in my PR, Stack.Constants doesn't only contain constants but also contains a bunch of helper functions for which it imports Stack.Types.Compiler, Stack.Types.Config, Stack.Types.PackageIdentifier and Stack.Types.PackageName. The problem is that these modules cannot in turn use any of the "actual" constants in Stack.Constants.

I propose that Stack.Constants become a "top-level" module that doesn't import any of the Stack.* modules, and that all the exports from Stack.Constants that depend on other Stack.* modules be moved elsewhere.

A few can even be deleted as they aren't used anywhere:

builtConfigFileFromDir
builtFileFromDir
configuredFileFromDir
defaultShakeThreads
userDocsDir
testBuiltFile
benchBuiltFile

For those "non-constant" exports that are currently used in only a single module, I'd propose that they could simply be moved inside that module.

After that there still remain the following "non-constant" functions that are used in more than one module:

distDirFromDir
distRelativeDir
hpcRelativeDir

Any ideas for those?

@sjakobi
Copy link
Member Author

sjakobi commented Jan 20, 2016

Maybe this could also be an opportunity to fix the topical overlap with the Paths section in Stack.Types.Config

@mgsloan
Copy link
Contributor

mgsloan commented Jan 20, 2016

I'm actually fine with the constants depending on types modules. Maybe we should just add a .hs-boot file?

Also, should probably wait for #1674 to be merged before digging into this. Not a huge deal, though, I don't think there'd be /that/ much conflict.

I agree that the Paths section in Stack.Types.Config should be merged into constants. I'm guessing it's due to similar module circularity issues.

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

No branches or pull requests

3 participants