rokucommunity / ropm

A package manager for the Roku platform.
MIT License
31 stars 5 forks source link

Please don't log every npm dependency which doesn't have the `ropm` keyword #39

Open elsassph opened 3 years ago

elsassph commented 3 years ago

My project heavily uses npm modules and ropm copy is uber verbose:

ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/color-support" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/tap-mocha-reporter" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/tap-mocha-reporter/node_modules/debug" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/tap-mocha-reporter/node_modules/ms" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/tap-mocha-reporter/node_modules/escape-string-regexp" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/tap-parser" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/diff" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/tap-yaml" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/minipass" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/events-to-array" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/yaml" because it does not have the "ropm" keyword
ropm: skipping prod dependency "/Users/philippe.elsass/dev/my-project/node_modules/unicode-length/node_modules/strip-ansi" because it does not have the "ropm" keyword
[... a hundred more lines of that]

That message REALLY isn't useful.

TwitchBronBron commented 3 years ago

So I do realize it's verbose, but many ropm developers (and publishers) aren't overly experienced with npm/ropm things, so it's helpful in my opinion to explain why a certain package didn't get copied to your rootDir.

That being said, perhaps we can introduce a logLevel flag, where the default would emit a single line:

143 prod dependencies were not copied because they do not have the `ropm` keyword

and then the more verbose loglevel would be where the full list of skipped packages would be printed.

elsassph commented 3 years ago

But what is it useful for? As a ropm user you will install various modules, usually following the instructions. Then you run ropm copy and you see which modules are copied, which is the useful information.

TwitchBronBron commented 3 years ago

What about this scenario?

ropm i lodash

Lodash is definitely not a ropm module. but ropm will "install it" just fine because ropm just initiates npm install behind the scenes. Also, ropm i runs ropm copy as well.

Now, when I run ropm copy, this is the only output I get right now:

C:\projects\RokuProject>npx ropm copy
ropm: skipping prod dependency "C:\projects\RokuProject\node_modules\lodash" because it does not have the "ropm" keyword

C:\projects\RokuProject>

I would much rather warn users about something not looking right, rather than making them investigate it themselves ("and you see which modules are copied") every time they run install.

elsassph commented 3 years ago

I'd suggest adding some ropm list maybe. But having this log on every ropm copy for every transitive dependency is a lot of noise.

georgejecook commented 3 years ago

I'm not a fan of the noise either, as it goes. I think this is a great solution:

That being said, perhaps we can introduce a logLevel flag, where the default would emit a single line:

143 prod dependencies were not copied because they do not have the `ropm` keyword
and then the more verbose loglevel would be where the full list of skipped packages would be printed.
pawelhertman commented 2 years ago

What about this scenario?

ropm i lodash

Lodash is definitely not a ropm module. but ropm will "install it" just fine because ropm just initiates npm install behind the scenes. Also, ropm i runs ropm copy as well.

Maybe that's the problem - maybe ROPM should inform that the package user is trying to install has no ropm keyword and will be ignored? Therefore even

143 prod dependencies were not copied because they do not have the ropm keyword

won't be necessary - what user should do with such information? I think it's useless