pivotal / LicenseFinder

Find licenses for your project's dependencies.
MIT License
1.73k stars 340 forks source link

Support node dependency groups #113

Closed taavo closed 9 years ago

taavo commented 10 years ago

Currently LicenseFinder is punting on the existence of npm dependency groups (e.g. "dependencies" vs "devDependencies"). It would be great to be able to exclude npm devDependencies the same way you can exclude bundler groups.

cc @nertzy

mainej commented 10 years ago

@taavo is #111 the proposed solution to this?

taavo commented 10 years ago

Nope. That pull request prevents a crash if you run LicenseFinder against a project with bundledDependencies, by removing our pretend support for them. It has no impact on dependency groups.

mainej commented 10 years ago

Ah, my confusion. So the proposal is that license_finder ignored_groups add dev should exclude Node dev dependencies? (And all the code that references ignored_bundler_groups should be changed to just ignored_groups;) Makes sense to me. :+1:

nertzy commented 10 years ago

Yeah something like that. Not sure if the group name should be "dev" or "development" or "devDependencies".

flavorjones commented 10 years ago

This seems like a dupe of #110. Or, have I missed a nuance?

taavo commented 10 years ago

This one is specifically about excluding dependency groups. As is I don't think there is a way to exclude groups in node like you can in ruby. #110 is about subdependencies, if I'm reading it correctly.

nertzy commented 10 years ago

Yes, @taavo is right. This issue is a feature request to "put things into groups so I can have more control as to what I choose to include". #110 is a bug report saying "don't forget that dependencies have sub-dependencies".

flavorjones commented 10 years ago

Roger, thanks guys!

On Mon, Oct 27, 2014 at 9:37 AM, Grant Hutchins notifications@github.com wrote:

Yes, @taavo https://github.com/taavo is right. This issue is a feature request to "put things into groups so I can have more control as to what I choose to include". #110 https://github.com/pivotal/LicenseFinder/issues/110 is a bug report saying "don't forget that dependencies have sub-dependencies".

— Reply to this email directly or view it on GitHub https://github.com/pivotal/LicenseFinder/issues/113#issuecomment-60592754 .

mainej commented 9 years ago

Fixed by #152.

nkoterba commented 9 years ago

So perhaps I'm missing something but I don't see how #152 addresses this question.

I'd like to EXCLUDE devDependencies from LicenseFinder when crawling my npm-based directories.

However, the "fix" in #152 appear to always include both dependencies and devDependencies.

Am I missing some configuration item or command line argument?

mainej commented 9 years ago

@nkoterba Have you run license_finder ignored_groups add devDependencies followed by license_finder report? If that's not working, you've found a bug. A minimal package.json which reproduces the problem would help.

nkoterba commented 9 years ago

@mainej Just verified that adding devDependencies as an ignored group does not filter out dev packages.

Keep in mind this is with npm/node. According to the README.md, ignored_groups is only supported with Bundler.

Not sure if the documentation is correct or outdated.

Here's a simple package.json file that should show the issues:

{
  "name": "test_ignored",
  "version": "1.0.0",
  "description": "Testing Ignored Groups with License_Finder",
  "dependencies": {
    "angular": "^1.4.3"
  },
  "devDependencies": {
    "gulp-rename": "^1.2.2"
  },
  "license": "ISC"
}

Here's what my dependency_decisions.yml file looks like after I ran the command you suggested above:

---
- - :ignore_group
  - devDependencies
  - :who: 
    :why: 
    :when: 2015-07-30 01:49:51.659344000 Z

And here's my output:

nk-mbp:webclient a$ license_finder
..........Dependencies that need approval:
angular, 1.4.3, MIT
gulp-rename, 1.2.2, MIT

If working, I believe we should only see angular there.

mainej commented 9 years ago

hmm.... @LukeWinikates any thoughts on this?

mainej commented 9 years ago

Actually, @LukeWinikates maybe there's no problem. @nkoterba, #152 hasn't been released to Rubygems yet. Does that explain things, or are you running off master from GitHub?

@flavorjones is it time for another version bump?

nkoterba commented 9 years ago

@mainej Yes, I'm using the RubyGems version.

It sounds like it may just be best to use the GitHub version until 2.0.5 is released to RubyGems.

LukeWinikates commented 9 years ago

@nkoterba I think you'll have success with the Github version. We're using it on my current project and successfully ignoring our devDependencies. Let me know if it doesn't work for you.

Thanks for pointing out the stale note in the README about Bundler being the only package manager with 'group' support.

nkoterba commented 9 years ago

@LukeWinikates I can confirm that the Github version does work ignoring devDependencies.

Thanks!