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

[#272] [#275] Travis improvements for generated project #279

Merged
merged 3 commits into from
Jan 6, 2019

Conversation

vrom911
Copy link
Member

@vrom911 vrom911 commented Jan 4, 2019

It doesn't work for Stack when the version of GHC is 8.0.2 because it uses old Stack, not the one that is configured, but it shouldn't work with the old version of Cabal (manually writing stack setup --no-terminal --install-cabal 2.2.0.1 doesn't help as well.

UPD: We decided not to add old GHC into Stack Travis matrix.

Resolves #272
Resolves #275

Checklist:

  • Keep code style used in the changed files (see style-guide for more details).
  • Use the stylish-haskell file.
  • Update documentation (README, haddock) if required.
  • Create a new test project using summoner and check that the changes work as expected.

@vrom911 vrom911 added help wanted CI CI tools support; project CI; build with different GHC, tools labels Jan 4, 2019
@vrom911 vrom911 self-assigned this Jan 4, 2019
@vrom911 vrom911 requested a review from chshersh January 4, 2019 21:50
Copy link
Contributor

@chshersh chshersh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's okay to not use --install-cabal option (looks like this option is not supposed to be used in production) and keep failing GHC-8.0.2. Basically, this is not our problem 🙂 We can't move forward if we will support everything... I guess experienced users know what they are doing and can introduce necessary manual changes to .travis.yml by themselves.

@@ -22,6 +23,10 @@ import Summoner.GhcVer (GhcVer)
defaultGHC :: GhcVer
defaultGHC = maxBound

-- | Default version of the Cabal.
defaultCabal :: Text
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice idea to move this into separate variable!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because our plans are to make it more flexible in the future :)

- ghc-${ghcV}
- cabal-install-head
|]
travisCabalMatrixItem (showGhcVer -> ghcV) = [text|- ghc: $ghcV|]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!!

@vrom911
Copy link
Member Author

vrom911 commented Jan 4, 2019

@chshersh I don't want to make not working settings, at least we shouldn't include this GHC version in Stack matrix and specify this behaviour explicitly somewhere with the reasoning. I will try to ask some people; probably somebody already solved this problem.

@chshersh
Copy link
Contributor

chshersh commented Jan 4, 2019

@vrom911 This is a very reasonable approach 👍 I agree, let's first ask, maybe something can be done with this. Otherwise - I have no idea how to fix this problem...

@vrom911
Copy link
Member Author

vrom911 commented Jan 4, 2019

Not sure it's the best place but:

https://twitter.com/vronnie911/status/1081317117977378816

(it's not really can be an issue to Stack, as they would say that this is feature)

UPD: Created Stack ticket:
commercialhaskell/stack#4488

@vrom911
Copy link
Member Author

vrom911 commented Jan 5, 2019

Things to do:

  • Add info message in TUI about old versions that won't be included into Stack matrix (if there are any)
  • Add info about the issue into README file

Copy link
Contributor

@chshersh chshersh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the change in general! Very careful work, can't think of anything that we also need to update 🤔

summoner-cli/src/Summoner/GhcVer.hs Outdated Show resolved Hide resolved
@@ -1,15 +1,29 @@
{-# LANGUAGE QuasiQuotes #-}
{-# LANGUAGE ViewPatterns #-}

{-| This module contains template for GitHub related docs:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This documentation is soooo niiice! 👏

@chshersh chshersh merged commit 089e313 into master Jan 6, 2019
@chshersh chshersh deleted the vrom911/272-cabal-travis branch January 6, 2019 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI CI tools support; project CI; build with different GHC, tools help wanted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants