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

Add support for Elm #700

Merged
merged 9 commits into from
Dec 11, 2015
Merged

Add support for Elm #700

merged 9 commits into from
Dec 11, 2015

Conversation

mrmurphy
Copy link
Contributor

Hoping I did this the right way. I just followed the example of the merged PR for supporting stylish-haskell.

Caveat: I don't like that the url for elm-format is hard-coded in the .travis.yml file, but the tool is new enough that it doesn't exist in brew yet.

@Glavin001
Copy link
Owner

I would accept if this works for Travis CI. Currently it fails: https://travis-ci.org/Glavin001/atom-beautify/builds/96061019

Looks great so far! Once tests are passing I'll take a closer look at the rest of the code, but I looked over it briefly and was definitely on the right track.

@mrmurphy
Copy link
Contributor Author

@Glavin001 Thanks! I solved the initial Travis failures, but now it appears to be failing on the titanium-style-sheets test. Any hints about how to proceed from here?

I haven't been able to run the tests, locally yet. I open the atom-beautifier folder in Atom, and then I run Window: Run Package Specs, but then a console pops up and I just see the error: AssertionError: missing path

@Glavin001
Copy link
Owner

I'll fix the unrelated failing tests right now -- Pretty Diff was likely updated again and broke those tests 😜 .

There is also a linting issue for your Elm code that could be fixed in the meantime 😄 .

@Glavin001 Glavin001 self-assigned this Dec 11, 2015
@Glavin001
Copy link
Owner

I pushed fixes for the failing tests. If you merge and I can see that your tests for Elm are passing, I will merge anyway 😄. Also be sure to run npm run docs and commit the documentation, too! Thanks.

@mrmurphy
Copy link
Contributor Author

Whoops! Sorry about that lint error! I've rebase on master, and I've fixed the linter error.

And wahoo! Looks like Travis passed!

@@ -0,0 +1,9 @@
module Main (..) where
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this file, or move to the test directory.

@mrmurphy
Copy link
Contributor Author

Thanks for those detailed comments! I've committed the docs, and I think I've addressed all of your comments.

@Glavin001
Copy link
Owner

Beautiful! Thank you for contributing!

Glavin001 added a commit that referenced this pull request Dec 11, 2015
@Glavin001 Glavin001 merged commit 29a0461 into Glavin001:master Dec 11, 2015
@Glavin001
Copy link
Owner

Published to v0.28.20

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

Successfully merging this pull request may close these issues.

2 participants