dimitri-xyz / haskell-coinbase-pro

A Haskell client for the Coinbase Pro API
MIT License
11 stars 6 forks source link

Code formatting and linting #13

Open ericpashman opened 4 years ago

ericpashman commented 4 years ago

Do you have a preference for a code format or linter config, @dimitri-xyz? Right now indentation and line lengths are inconsistent from code block to code block, and I'd like to pretty everything up. The codebase could also benefit from some basic linting.

The only thing I have much of preference about is that lines wrap at 80 characters or fewer, as I'm one of those people who does actually code in an 80-character-wide frame. If you have something in particular you want to use, let's use it. Otherwise we can start with the default configurations of HIndent and HLint.

ericpashman commented 4 years ago

FYI, this new code formatter seems to be getting some traction as an alternative to HIndent.

This is something that can be added to our future CI config at some point.

dimitri-xyz commented 4 years ago

We will need to hash this out.

I really don't like Ormolu as it breaks conventions (specially the ones it has to justify breaking on the page link you sent: commas and arrows). I played with it and I think it tends to make type signatures much less readable. I vote against it.

I am also against setting the 80 width limit on lines. I think that made sense in the past (variable names in fortran were usually a single character), but nowadays our monitors allow for much wider limits (e.g. 120). It seems programmers always strive to have short lines and only make for longer lines if the shorter one is also not a good choice. So, I don't think setting a hard limit is useful. I think this is specially bad in haskell if we want to use long identifier names, which I think are more important.

I think linting would be useful, but, again, I don't think it should be mandated. For example, I have seen a few situations where I want to have extra parenthesis in pattern matching, just so that all cases line up. I actually think this makes the code simpler to read, but the simple linter rules don't take the formatting into account.

ericpashman commented 4 years ago

OK, nix automated linting and formatting.

But if you're open to linting commits, I'm in favor of making one every now and then. HLint makes some suggestions I don't find particularly helpful, but it also makes plenty that I do. Examples of things HLint finds in the current codebase that I would fix: truly redundant groupers, needless explicit applications (f $ x), instances of using data where newtype can be used. (Slightly more controversially: 10 occurrences of liftM!) It's helpful for catching unused dependencies and language extensions.

As for formatting, if there's a formatter you want to use, let's use one. I really don't care which. I propose a non-aggression pact on the line-length front along these terms: I won't insist on breaking all lines at 80 characters if you'll let me toss in some newlines here and there (like, say, one newline somewhere towards the middle of each 100-character deriving line).