-
Notifications
You must be signed in to change notification settings - Fork 28
add links for depedencies, extra licenses, and output as file #20
base: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 60
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I made a few comments. Feel free to counter me on any of them. :)
@@ -17,12 +17,14 @@ defmodule Licensir.Guesser do | |||
Map.put(license, :license, conclusion) | |||
end | |||
|
|||
defp guess(file, ""), do: guess(file, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember correctly, there shouldn't be any case where the 2nd argument is an empty string. So I think if it falls into this case we should fix the root cause than handling it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a case of a dependency with no license which results in an empty license column in the final rather than "Undefined" if this line is present.
end | ||
|
||
defp render(rows, opts) do | ||
if Keyword.get(opts, :only_license), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the short hand if: ..., do: ..., else: ...
are discouraged for multi-line conditionals per the popular code style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are single lines though
How does it look now? |
Hope others find this useful... Also tweaked the logic so as to properly recognise at least all the deps in my project :)