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

bears/haskell: Add GhcModBear #1223

Closed
wants to merge 2 commits into from

Conversation

vijeth-aradhya
Copy link
Member

Add GhcModBear for syntax checking
for Haskell files (.hs)

Closes #297

@gitmate-bot
Copy link
Collaborator

Thanks for your contribution!

Reviewing pull requests take really a lot of time and we're all volunteers. Please make sure you go through the following check list and complete them all before pinging someone for a review.

As you learn things over your Pull Request please help others on the chat and on PRs to get their stuff right as well!

"""
Syntax checking with ghc for haskell files (.hs)

Check out https://hackage.haskell.org/package/ghc-mod for more information!
Copy link
Member

Choose a reason for hiding this comment

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

the link should be between <>

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright! 😄

r'(?P<message>.+)')
class GhcModBear:
"""
Syntax checking with ghc for haskell files (.hs)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if we need (.hs) here

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm I need to specify the file extension
Any ideas on how? []?

Copy link
Member Author

Choose a reason for hiding this comment

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

Any reason why CircleCI is failing? :/

Syntax checking with ghc for haskell files.

See <https://hackage.haskell.org/package/ghc-mod for more information>
for more information!
Copy link
Member

Choose a reason for hiding this comment

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

double "for more information"

@sims1253
Copy link
Member

your builds are failing :(

main = do
print(lastButOne(["asd", "ASd"]))
"""

Copy link
Member

@Mixih Mixih Dec 30, 2016

Choose a reason for hiding this comment

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

hmm let's use a test class with a generate_skip_decorator so we know whether it's skipping because of a depend or some internal logic error. See http://api.coala.io/en/latest/Developers/Testing_Bears.html

Copy link
Member

@Mixih Mixih Dec 30, 2016

Choose a reason for hiding this comment

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

actually that is unnecessary skipdecorator is built in to that test class... Just install it properly in ci

Copy link
Member

Choose a reason for hiding this comment

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

No really necessary IMO.

@@ -29,3 +29,4 @@ vim-vint~=0.3.10
vulture~=0.10.0
yamllint~=1.5
yapf~=0.14.0
dependency_management~=0.2.0
Copy link
Member

Choose a reason for hiding this comment

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

Please don't add this, it can cause all kinds of dependency problems as we've already inherited it from coala. I see you have a pull there, Please remove this when it gets accepted.

@sils
Copy link
Member

sils commented Jan 6, 2017

@vijeth-aradhya you'll have to update coala in requirements.txt to version >=0.10.0.dev20170106003439 to pick up the change you did

AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'coala-devel@googlegroups.com'}
LICENSE = 'AGPL-3.0'
ASCIINEMA_URL = 'https://asciinema.org/a/97863'
Copy link
Member

Choose a reason for hiding this comment

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

FWIW it looks like there's spaces missing in the message, is that like that in the output of the tool or a bug of ours somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

spaces missing?! What do you mean :o

Copy link
Member

Choose a reason for hiding this comment

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

from the literal ‘3’In the first no space before In

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm i'll check it out now 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

This is ghc-7.10.3 output on my local machine for the first testfile

/GhcModBearTests/testfile1.hs:2:17:
    No instance for (Num String) arising from the literal ‘3’
    In the first argument of ‘putStrLn’, namely ‘3’
    In the expression: putStrLn 3
    In an equation for ‘main’: main = putStrLn 3

Looks to me that wherever there is \n, it's just joining the strings

Copy link
Member

Choose a reason for hiding this comment

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

can you file an issue for linter in coala?

Copy link
Member Author

Choose a reason for hiding this comment

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

It'll be nice if we make it prettier I guess 👍 Hmm anyway we can add a \n there?!

Copy link
Member Author

@vijeth-aradhya vijeth-aradhya Jan 6, 2017

Choose a reason for hiding this comment

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

Oh wait my bad, that is ghc output, this is ghc-mod-5.6.0 output
There is nothing wrong in our Linter - it's their output which is like that 👍

~/GhcModBearTests/testfile1.hs:2:17:No instance for (Num String) arising from the literal ‘3’In the first argument of ‘putStrLn’, namely ‘3’In the expression: putStrLn 3In an equation for ‘main’: main = putStrLn 3

Copy link
Member

Choose a reason for hiding this comment

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

cool, file a bug upstream ok?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup I did, here 👍

@sils
Copy link
Member

sils commented Jan 6, 2017

tests are much better, I feel like the extra round of review really added some quality here 👍 I hope this wasn't too frustrating :/. If you change the requirements as indicated above I think it'll work and we can really finally merge it.

Add ghc-mod feature to coala for syntax checking for
haskell files (.hs)

Closes coala#297
Upgradation of dependency_management version from
0.1.7 to 0.2.0 is major modification here

Related to coala#297
@gitmate-bot
Copy link
Collaborator

Comment on 11845d8.

Shortlog of HEAD commit isn't in imperative mood! Bad words are 'Updated'

GitCommitBear, severity NORMAL, section commit.

@vijeth-aradhya
Copy link
Member Author

Travis still weirdly failing .. 😞

@jayvdb
Copy link
Member

jayvdb commented Apr 28, 2017

Completed at #1267

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

Successfully merging this pull request may close these issues.

None yet