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

Depend on semigroups #140

Merged
merged 3 commits into from
Feb 2, 2018
Merged

Depend on semigroups #140

merged 3 commits into from
Feb 2, 2018

Conversation

LightAndLight
Copy link
Contributor

@LightAndLight LightAndLight commented Jan 12, 2018

Conditional dependency on semigroups causes some problems for tools that don't know now to deal with ifs in cabal files. I can't build hegdgehog on the ghc7102 or ghc7103 package sets, because the tools don't know how to account for the conditional dependency. It's okay to just always depend on semigroups - see https://qfpl.io/posts/semigroups/

Please squash this PR when you merge it :)

@LightAndLight
Copy link
Contributor Author

I'm not sure why the tests have failed. I'll look into it later

@gwils
Copy link
Contributor

gwils commented Jan 12, 2018

It looks like the jobs timed out.

@thumphries
Copy link
Member

That's interesting, which tool fails to understand Cabal files? cabal2nix or something? Does that cause problems more widely? Lots of packages use conditionals...

This seems harmless, we should probably merge it given the additional context.

(I don't have the perms, you will have to wait a little longer)

@nhibberd
Copy link
Collaborator

It's okay to just always depend on semigroups

This is still adding a redundant dependency for > 8

@LightAndLight
Copy link
Contributor Author

@thumphries Yeah, it's cabal2nix. It currently just ignores conditional dependencies.
@nhibberd Okay. Is one of the goals of this project to minimize the number of external dependencies? I use GHC8+, and the thought of an extra dependency doesn't bother me.

@thumphries
Copy link
Member

Yeah, it's cabal2nix. It currently just ignores conditional dependencies.

If this is the case it's broken, that situation is kinda untenable. Could this be fixed on the nix / pkgs side by editing the derivation? Is that the intended usage pattern?

In any case this is a Cassava kind of situation, we'll merge this to resolve it, just waiting on some messed up repo permissions to do so.

@LightAndLight
Copy link
Contributor Author

In any case this is a Cassava kind of situation

Yeah I guess it is, although I wouldn't kick up such a fuss if you said no ;)

@thumphries thumphries merged commit d8f2fde into hedgehogqa:master Feb 2, 2018
@thumphries
Copy link
Member

Sorry that took so long. Perms sorted now

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.

None yet

4 participants