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

Allow 'github' shorthand for extra-deps (fixes #3873) #3890

Merged
merged 2 commits into from Mar 12, 2018

Conversation

Projects
None yet
2 participants
@mitchellwrosen
Contributor

mitchellwrosen commented Feb 25, 2018

First, sorry to @lhcopetti for stealing this one - I neglected to notice you were interested in doing this.

This patch adds support for this syntax of extra-deps:

- github: commercialhaskell/rio
  commit: 09654f9fcbdcd96d0f5102796b32fdac5da7260e

with an additional optional subdirs field. The above would resolve to this archive:

https://github.com/commercialhaskell/rio/archive/09654f9fcbdcd96d0f5102796b32fdac5da7260e.tar.gz

Please let me know if I should tweak anything, including adding more tests. Thanks!

@snoyberg

Just a few minor comments, looks great! Thanks for writing this. Sorry for the delay in review, I didn't see it until now.

@@ -150,7 +150,7 @@ instance IsString WarningParserMonoid where
-- Parsed JSON value with its warnings
data WithJSONWarnings a = WithJSONWarnings a [JSONWarning]
deriving Generic
deriving (Eq, Generic, Show)

This comment has been minimized.

@snoyberg

snoyberg Mar 11, 2018

Contributor

Is this change necessary for the PR, or just to help with debugging?

This comment has been minimized.

@mitchellwrosen

mitchellwrosen Mar 11, 2018

Contributor

Eq and Show are required for test code like:

decode' contents `shouldBe` Just (WithJSONWarnings expected [])
@@ -269,6 +270,26 @@ instance subdirs ~ Subdirs => FromJSON (WithJSONWarnings (PackageLocation subdir
, archiveHash = msha'
}
github = withObjectWarnings "PLArchive" $ \o -> do

This comment has been minimized.

@snoyberg

snoyberg Mar 11, 2018

Contributor

It should probably say PLArchive:github or something like that instead, this is just used for error messages, and it would be useful to know what part of the code failed the parsing.

This comment has been minimized.

@mitchellwrosen
@@ -0,0 +1,94 @@
{-# LANGUAGE OverloadedStrings #-}
module Stack.Types.BuildPlanSpec where

This comment has been minimized.

@snoyberg

snoyberg Mar 11, 2018

Contributor

Nice! 👍

@snoyberg

Awesome, thanks!

@snoyberg snoyberg merged commit 4a14034 into commercialhaskell:master Mar 12, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mitchellwrosen

This comment has been minimized.

Contributor

mitchellwrosen commented Mar 12, 2018

Of course!

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