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

Fix a few more issues shown by mix credo #604

Merged
merged 1 commit into from Dec 28, 2018

Conversation

Projects
None yet
2 participants
@michaelstalker
Copy link
Contributor

michaelstalker commented Oct 26, 2018

This PR doesn't fix more than a handful of issues, but here's what it does:

  • Ignore a readability issue. We want a module name to have underscores when designating an Elixir version.
  • Remove deep nesting.
  • Allow IO.inspect/1, but in a way that mix format will not undo. mix format added a space before credo:disable-for-previous-line, so we needed a different way to ignore that IO.inspect/1 call.

There are a few readability issues flagged by mix credo which involve missing @moduledocs. Would you like me to work on documenting those modules, or would you prefer @moduledoc false? Additionally, mix credo is flagging a 18 TODOs and one FIXME. Would you like me to convert those to GitHub issues, and remove the TODO and FIXME comments from the codebase? I can do one or both of those things in separate PRs if you'd like me to.

@rrrene

This comment has been minimized.

Copy link
Owner

rrrene commented Oct 30, 2018

@michaelstalker Could you rebase against the upstream master?

Fix a few more issues shown by mix credo
* Ignore a readability issue. We want a module name to have underscores
  when designating an Elixir version.
* Remove deep nesting.
* Allow IO.inspect/1, but in a way that `mix format` will not undo.
  `mix format` added a space before credo:disable-for-previous-line, so
  we needed a different way to ignore that IO.inspect/1 call.

@michaelstalker michaelstalker force-pushed the rentpath:feature/fix_readability_issues branch from 5fbc4d1 to 435e793 Oct 31, 2018

@michaelstalker

This comment has been minimized.

Copy link
Contributor

michaelstalker commented Oct 31, 2018

@rrrene Done! 😄

@rrrene rrrene merged commit 7251a79 into rrrene:master Dec 28, 2018

1 of 2 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rrrene

This comment has been minimized.

Copy link
Owner

rrrene commented Dec 28, 2018

@michaelstalker Thank you! 👍

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