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

Don't use full-path when passing it to Credo #4

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

paulswartz
Copy link

Credo seems to check the whole repository when a full path is given to it. This PR changes that to only pass the relative path.

It additionally relaxes the check for line numbers: this allows it to catch changes like "pipe chain should start with a raw value" which tend to be triggered by different line from what's in the patch.

Errors like "Pipe chain should start with a raw value" tend to be reported on
a slighty different line from the diff. This picks them up by checking to see
if the line is "close" to the diff line.
Passing the full path makes Credo take forever to scan the file.
@carakan
Copy link
Owner

carakan commented Mar 23, 2017

Thanks @paulswartz for this PR's, I'll take a look your changes shortly.

@carakan
Copy link
Owner

carakan commented Apr 10, 2017

@paulswartz I reviewed your changes, that seems pretty good, but I see a lot of new messages per commit. I pushed a new branch develop in that tagging version 0.0.5 and testing and comparing with 0.0.4 the new version adds new messages, I need to compare if your new changes makes a better handling of changes than older version or adds unrelated messages.

@paulswartz
Copy link
Author

It definitely adds more messages: what it tries to do is find errors on adjacent lines. Consider this diff:

   defp do_build_matcher({key_str, value_str}, acc) do
     values = integer_values(value_str)
+    |> Enum.map(& &1)
     matchers_for_values(acc, key_str, values)
   end

The only line in the diff is fine. However, adding that line causes a Credo issue on the previous line (pipe chain should start with a raw value). My branch tries to find those nearby issues and include them as messages, since they are related to the commit.

@carakan carakan mentioned this pull request May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants