Skip to content

Port to spago (CI) #181

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

Closed
wants to merge 3 commits into from
Closed

Port to spago (CI) #181

wants to merge 3 commits into from

Conversation

emiel
Copy link
Contributor

@emiel emiel commented Apr 26, 2021

Here is my attempt to continue with the work done here #127 in replacing bower/pulp with spago. I focused on getting CI setup to build and test the exercises (just practice exercises for now). The approach is more or less similar to what was being done for bower. Spago uses two files packages.dhall (defines package-set) and spago.dhall (project dependencies).

  • All exercises should use the same package set so that we can share a .spago and output directory.
  • We gain some additional flexibility in that spago.dhall can vary per exercise. This is nice because we can whittle down the exercise dependencies to the bare minimum and we save students some diskspace. Also spago has started warning about unused dependencies... a topic for when we tackle porting the exercises to 0.14.x.

Please have a look (the last commit contains the interesting bits) and see if you think this approach is worth pursuing. If so I can continue to update the docs and add the finishing touches.

NOTE: Without sharing the output and .spago directories the CI run takes around 7 minutes...

@connorads @aimorris

@emiel
Copy link
Contributor Author

emiel commented Apr 26, 2021

uses: purescript-contrib/setup-purescript@main
with:
purescript: ${{ matrix.version }}
spago: "0.19.2"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pin spago to "0.19.2" because later versions don't appear to run the tests. :(

Copy link
Member

Choose a reason for hiding this comment

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

This will be good to include in the documentation :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been resolved in 0.20.2 and is no longer an issue.

@iHiD iHiD requested a review from ErikSchierboom April 26, 2021 21:18
@iHiD
Copy link
Member

iHiD commented Apr 26, 2021

@emiel Thank you 💙
@ErikSchierboom will take a look at this later in the week when he's back working. Before then hopefully some @exercism/purescript people can take a look! :)

@iHiD iHiD requested a review from a team April 26, 2021 21:19
@SleeplessByte
Copy link
Member

The travis is failing because it's still using the old configlet

@emiel
Copy link
Contributor Author

emiel commented Apr 27, 2021

The travis is failing because it's still using the old configlet

Thanks for having a look @SleeplessByte. It is my understanding Excercism is migrating completely to GitHub CI and thus that is where my focus is now. I'd rather tackle configlet in another PR because I guess there is more work to do there. ;)

@SleeplessByte
Copy link
Member

Totally. It was mostly informational and why I approved.

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

I don't know anytthing about purescript or dhall, but it looks good to me!

@declarationperfume
Copy link
Contributor

I concur

@emiel emiel mentioned this pull request May 9, 2021
@emiel
Copy link
Contributor Author

emiel commented May 9, 2021

Closing in favor of #185

@emiel emiel closed this May 9, 2021
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.

5 participants