Comments on the Diligent Readme and functionality

foo.md

Intro

I'm assuming you are asking me for my opinion because you want to publish the project and you want to know if it complies with the Go project conventions.

About the README

I think the README is missing some important information and I would introduce some of the topics in a different order.

1. I miss an "Author" section at the end with your name and contact.

2. I miss a "License" section at the end with the license type and pointing to the license file.

3. I miss a "Contributing" section at the end with instructions on how to contribute.

4. The initial description and the "Language and Dependency Manager Support" are OK, but then I will insert an "Install" section before the "Usage" section.

   The "Running Locally" section you have near the end looks like an "Install" section but it is giving too much information about the GOPATH details that everybody using Go already knows and the actual install instruction are wrong. Just write an "Install" section with the command to install, like this:

   bash$ go get -u github.com/senseyeio/diligent/cmd/diligent
   

5. Then we need a usage section, but I think the current one has some problems:

   • I don't think to start with the docker usage is a good idea because it complicates things a lot with the whole volume thing and it is currently not working:

     bash$ ls diligent
     [...]
     Gopkg.lock
     [...]
     bash$ docker run -v /diligent:/dep senseyeio/diligent Gopkg.toml
     [...]
     open Gopkg.toml: no such file or directory
     

     So I will focus the usage section in how to use the command without docker for now.

   • I will show the basic command with a real execution of the code everyone can replicate, using its own repo, for instance:

     bash$ diligent ${GOPATH%%:*}/src/github.com/senseyeio/diligent/Gopkg.lock
     unknown file
     

     This should work but fails, so I will first fix this.

   • The way to run the above command in the current version is:

     bash$ cd ${GOPATH%%:*}/src/github.com/senseyeio/diligent && diligent Gopkg.lock 
     Failed to determine license for github.com/inconshreveable/mousetrap: failed to find license
     Failed to determine license for github.com/ryanuber/go-license: failed to find license
     github.com/pelletier/go-toml -> MIT License
     github.com/spf13/cobra -> Apache License 2.0
     github.com/spf13/pflag -> BSD-3-Clause
     dependency github.com/pelletier/go-toml has license MIT which is not in your license whitelist
     

     Now I will show this and explain what is the role of the whitelist mentioned in the last line, given that the command didn't even mention a whitelist on its invocation parameters. This explanation should be continued with "Whitelisting" sections as a subsection, where the flags are explained.

6. Then instead of the "Running Locally" I will explain how to use it with Docker, simply repeating one of the most complex examples above but running it with Docker. I will also briefly explain that we need to pass the directory as a volume to Docker.

7. I think the "Exit codes" section at the end is OK.

8. In general, I'm missing an explanation about how it uses the exit code 0 as a sign of compliance to the whitelist, as the stdout/stderr are being used for other (albeit related) reporting. Maybe you can explain that better, with an example using a bash if and then showing the exit values (echo $?) after the first examples of each kind (without whitelist, with a GPL-3.0 whitelist and the same with permissive.

Functionality

I have been using the program for a bit, I think the functionality is OK.

I would have split the functionality of listing licenses and checking them in two commands: a list command that only list the subprojects and their licenses and a whitelist command that only reports if the subprojects comply with the whitelist or not, that returns a single line ("pass" or "fail", in addition to the exit value).

I miss an example of the multiple output formats or just showing the help of the command to show that it can do other things too.

Code

I haven't had time to look at the code.

I see you added a Godoc badge, which generally means "this code is not just a command, it is also a library that you can use". I also see you have documented some methods and types extensively, which makes me think this is definitely a lib in addition to a command.

If that is true, then I would add package documentation to the packages, especially the main one, explaining how the different types are related and their purpose.

In general, I see lots of long names (NewLicenseGetter, WebLicenseGetter, ReplaceCategoriesWithIdentifiers...) which is something Gophers avoid.