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

Update to Elm 0.18 #15

Merged
merged 2 commits into from
Nov 14, 2016
Merged

Update to Elm 0.18 #15

merged 2 commits into from
Nov 14, 2016

Conversation

Anahkiasen
Copy link
Contributor

@Anahkiasen Anahkiasen commented Nov 13, 2016

I didn't find a CONTRIBUTING.md so I tried to update the repository the best I could.

Few things to note:

  • Latest version of elm-test already comes with a Node runner so I removed the manual compilation to JS
  • There were two or three ways the tests seemed to be runnable from: npm test, make test there was also a prepare_test file, so I got a bit confused. I tried to streamline the process the best I could with what the tools allow in their latest versions
  • I tried to run elm-format on the repository to cleanup my changes but the latter formats to 4 spaces as indentation instead of the 2 configured here so I thought it best to not touch that part
  • I wasn't sure what to do with Http.decodeUri (and encode) now returning Maybes. I've just added a basic Maybe.withDefault "" where relevant but you may have a better way
  • A lot of things changed in elm-test generally, I've tried to update the test suite in a way that made sense but I think it could be cleaned up way more – I'm new to Elm but it appears a lot of the things you did manually (generating test suites and stuff) is now handled built-in by elm-test so, might want to take a look at it
  • I saw a CI badge in the README but no CI configuration file nor automated validation of pull requests so, not sure what to do on that part

@sporto
Copy link
Owner

sporto commented Nov 14, 2016

Great, thanks for this

@sporto sporto merged commit cb7e748 into sporto:master Nov 14, 2016
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.

2 participants