nodeshift-archived / license-reporter

license-reporter is a tool that gathers licenses for project's dependencies and produces a output in XML, JSON, YAML and HTML format.
Apache License 2.0
13 stars 10 forks source link

🎨 Minor suggestions for file names and html output #239

Closed grdryn closed 6 years ago

grdryn commented 6 years ago

These are 2 things that I figured a PR was an easier way to propose/discuss, rather than creating an issue to get your feeling about them. Below are the commit messages:

🎨 Call it licenses.html instead of license.html

The xml and css (maybe others?) use the plural 'licenses.<extension>' in
their file names. This just changes the html file to match those.

and

🎨 Put packageName in "Package Artifact" col in html

Before this change, it would be in the "Package Group" column. I guess the
2 columns would map to a groupId and artifactId in maven. Given that we
don't have groupIds in npm dependencies, I think the name fits better in
the artifact column.

Let me know what you think!

grdryn commented 6 years ago

@helio-frota let me know what you think please! 🙏

grdryn commented 6 years ago

I'll fix the test if you think the change is a good idea! :)

helio-frota commented 6 years ago

@grdryn sorry the delay : ] I'll look now

helio-frota commented 6 years ago

@grdryn

About the file name in plural this is not problem at all.

I guess the 2 columns would map to a groupId and artifactId in maven. Given that we
don't have groupIds in npm dependencies, I think the name fits better in
the artifact column.

I agree with that, I think we was only following the convention of the documentation (about the names used by maven).

The question is: Can we change this ? @lgriffin ? If ok, then just need to fix the test and merge : ]

lgriffin commented 6 years ago

We have full freedom here as the HTML and associated CSS were designed with Java in mind. We want to mimic the look and feel of the report but the column names should reflect what makes sense for our language. So we should be naming them appropriately. Similarly the plural usage for licenses is again in our remit. So +1 on both suggestions to me

helio-frota commented 6 years ago

@grdryn we can merge after fix the test. Thanks!

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 90.316% when pulling 73854b743c7d145ca39f386081f25a5a112ebb8e on grdryn:minor-suggestions into a58b9b570414fba843e4eefad0737595535f7a03 on bucharest-gold:master.

helio-frota commented 6 years ago

@grdryn thanks!