Skip to content

hlint: Improve existing code based on suggestions #355

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
30 tasks done
rbasso opened this issue Sep 24, 2016 · 14 comments
Closed
30 tasks done

hlint: Improve existing code based on suggestions #355

rbasso opened this issue Sep 24, 2016 · 14 comments
Assignees

Comments

@rbasso
Copy link
Contributor

rbasso commented Sep 24, 2016

To promote good coding practices, we are changing README.md to recommend contributors to run HLint before every submission. #354

That may help in the future, but running hlint on master indicates that there is also a lot of work to be done right now. The following is a list of files to inspect and decide if we should accept HLint's suggestions:

@rbasso rbasso self-assigned this Sep 24, 2016
@petertseng
Copy link
Member

When this is all said and done perhaps we should consider adding it to travis. On a quick look... it seems a bit strange that there is a parse error on robot-name/src/Example.hs ...?!

@petertseng
Copy link
Member

We may also consider whether we want to make everything required if we put it in Travis. It looks like hlint has various levels (suggestion, warning, error)

@rbasso
Copy link
Contributor Author

rbasso commented Sep 24, 2016

there is a parse error on robot-name/src/Example.hs

I saw that, but I didn't inspect it carefully.

When this is all said and done perhaps we should consider adding it to travis...

We may also consider whether we want to make everything required if we put it in Travis. It looks like hlint has various levels (suggestion, warning, error).

Depending on how we can fine tine the suggestions, that would be great. Sometimes it gives suggestion I would like to ignore...

ps: My plan was to finish the PRs today, but I'll have to leave it for tomorrow.

@petertseng
Copy link
Member

On a quick look... it seems a bit strange that there is a parse error on robot-name/src/Example.hs

We are not alone, I suppose. ndmitchell/hlint#216

@rbasso
Copy link
Contributor Author

rbasso commented Sep 25, 2016

list-ops is a huge problem for our plan of running hlint in Travis...

@petertseng
Copy link
Member

list-ops is a huge problem for our plan of running hlint in Travis...

Will telling hlint to ignore it be an acceptable solution? As we might see in http://community.haskell.org/~ndm/darcs/hlint/hlint.htm

I added a {-# ANN foldr "HLint: ignore Use foldr" #-} and it ignored that particular warning

@rbasso
Copy link
Contributor Author

rbasso commented Sep 25, 2016

Great finding! 👍

Will telling hlint to ignore it be an acceptable solution?

Even if we don't run hlint in Travis, it seems a good idea! 😄

@rbasso
Copy link
Contributor Author

rbasso commented Sep 25, 2016

With the ability to suppress specific HLint's suggestion, I believe we'll be able to run it in Travis.

First we have to keep changing and supressing unwanted suggestions until we get no hints.

Later, we'll have to find a way to install it in Travis. I didn't look at at, but I bet this will be the hardest part.

@petertseng
Copy link
Member

Later, we'll have to find a way to install it in Travis. I didn't look at at, but I bet this will be the hardest part.

Surprisingly (?), this appears not to be so hard:

https://github.com/petertseng/xhaskell/commit/6682e7475f3c167c10bde2d70dbd1db656f19932

resulting in:

https://travis-ci.org/petertseng/xhaskell/jobs/162545955

@petertseng
Copy link
Member

Curiosity - this appears to have installed a different version than what I have locally. You see that I got it from the apt repos. The Travis workers run Precise, don't they... http://packages.ubuntu.com/source/precise/hlint seems to say that would be 1.8.24, whereas the version I have installed locally is 1.9.37. The suggestions given are different (on Travis, there is a liftM suggestion that I don't have locally, and the pattern parse error is missing).

So it may be better to have stack install a later version. Hopefully that is not that much harder...?

@rbasso
Copy link
Contributor Author

rbasso commented Sep 25, 2016

Hopefully that is not that much harder

A little. Another problem is that it may increase initial building times.

@petertseng
Copy link
Member

petertseng commented Sep 25, 2016

Another problem is that it may increase initial building times.

Oh, you are right, this adds almost 8 minutes to the build time

https://travis-ci.org/petertseng/xhaskell/jobs/162546960

https://github.com/petertseng/xhaskell/commit/9bf276f16c22111a39d9d6d4f8e55c1132a5c477

@rbasso
Copy link
Contributor Author

rbasso commented Sep 25, 2016

8 minutes?!

I'll run some more tests later, but I'm still optimistic about it. :)

@rbasso
Copy link
Contributor Author

rbasso commented Sep 28, 2016

I guess we are done with fixing all hlint suggestions!

Thanks for everything, @petertseng!

$ hlint --version
HLint v1.9.37, (C) Neil Mitchell 2006-2016
j
$ hlint exercises
No hints (5 ignored)

@rbasso rbasso closed this as completed Sep 28, 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

No branches or pull requests

2 participants