The Credo experiment

[joshk]

This PR adds credo and addresses the most important issues (and some low hanging fruit) raised when using --strict.

This also adds it to our CI, but it doesn't fail it for now.

I've also modified some of the settings:

• disabled {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []} as I don't know if I agree with this, at the very least we should discuss and decide if we want this enforced. (this check, if enabled, would require us to remove () from functions which don't accept any arguments)
• disabled {Credo.Check.Refactor.CondStatements, []} as we have two 2-condition cond statements in NervesHub.Extensions which are useful to keep around. If we want to enforce this then we might need to change these until it makes sense switching back to using cond for them.
• customized {Credo.Check.Refactor.Nesting, [max_nesting: 3]}, the default is 2 but we have enough 3 level nesting that I didn't feel like going down that path right now.
• customized Credo.Check.Warning.MissedMetadataKeyInLoggerConfig to tell it that we do use :all in runtime.exs
• customized Credo.Check.Readability.ParenthesesOnZeroArityDefs to require parentheses (the opposite of its default)
• enabled Credo.Check.Warning.LeakyEnvironment to warn us if we are passing the full environment from the parent process when running system commands.
• enabled Credo.Check.Readability.ImplTrue as @jjcarstens hates it when @impl true is used
• and finally, enabled Credo.Check.Readability.MultiAlias as the use of alias NervesHub.{Deployments, Devices, Products, ...} is a code smell (@jjcarstens can swear to this)

We have only readability and software design suggestions issues to address, or ignore. These primarily consist of:

• Modules should have a @moduledoc tag
• Nested modules could be aliased at the top of the invoking module.
• and Found a TODO tag in a comment

I think we should address all of these, but we can instead leave these for another PR.

[lawik]

Good start!

I agree with skipping parenthesis for zero-argument functions personally. I believe the norm for Nerves-projects are to add the () so I think the consistency is probably worth the lacking aesthetics :D

I'll let others weigh in as well but I think this is the right direction. I think there are some things that Credo won't enforce that is preferred in Nerves projects. If/when we see those it may be worth making a Credo-integration for that. So we have something common to all Nerves Project projects.

[nshoes]

disabled {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []}

I abhor no parens 😆 I like the rule that there are always parens, regardless of arity. Aesthetically this makes more sense to me.

disabled {Credo.Check.Refactor.CondStatements, []} as we have two 2-condition cond statements in NervesHub.Extensions which are useful to keep around

This seems a little silly to me. Why not refactor those two-condition conds?

changed {Credo.Check.Refactor.Nesting, [max_nesting: 3]}, the default is 2 but we have enough 3 level nesting that I didn't feel like going down that path right now.

Yep, absolutely 😆 3 is fine for now.

[joshk]

disabled {Credo.Check.Refactor.CondStatements, []} as we have two 2-condition cond statements in NervesHub.Extensions which are useful to keep around

This seems a little silly to me. Why not refactor those two-condition conds?

cond makes a ton more sense in these two places because when another Extension version is released we can just slot it in there. So there is part of me that is like "why change this when it will change back in the not so distant future?"

And yet I also see your point, so I'm kinda in two minds.

I'll defer to @jjcarstens for the final decision 😄

[joshk]

@jjcarstens you might be interested in us enabling Credo.Check.Readability.ImplTrue, which warns against using @impl true, which I know you hate.

[joshk]

I've gone ahead and enabled MultiAlias, ImplTrue, and LeakyEnvironment credo checks, as well as fixing the issues associated with them.

[joshk]

@nshoes @lawik @jjcarstens please reread the PR description as I've updated it to reflect some recent changes (a few rules enabled, and a few rules customized)