Here's a similar discussion on the relative merits of shrinkwrap from 2014. https://markmail.org/thread/osbvx53d3l5s6fsj
@purplecabbage risingj.com On Fri, Sep 14, 2018 at 2:04 PM Chris Brody <chris.br...@gmail.com> wrote: > A quick try of #6 worked for me, you can see the results in my > cordova-cli WIP PR at: https://github.com/apache/cordova-cli/pull/326 > > It is probably not the best: I simply edited npm-shrinkwrap.json by > hand, then npm install would update it further. > > Resolves npm audit issues, runs on Node.js 4, does not need possibly > breaking change from insight@0.10.x. > > I will try to do a better job next week, at least we know #6 is possible. > On Fri, Sep 14, 2018 at 4:52 PM <raphine...@gmail.com> wrote: > > > > I'd really like to try #6. If that does not work as expected, we can > still > > go with #2. > > > > Jan Piotrowski <piotrow...@gmail.com> schrieb am Fr., 14. Sep. 2018, > 21:47: > > > > > #2 sounds absolutely fine to me as this dependency is in cordova-cli > > > which is only used on developer machines, not included in any deployed > > > packages. > > > > > > Besides: Cordova has been shipping software with `npm audit` like > > > issues for ages and I don't think there has been a "totally > > > unacceptable in all cases" vote result on that. > > > > > > -J > > > > > > 2018-09-14 21:31 GMT+02:00 <raphine...@gmail.com>: > > > > 6. Use manually edited npm-shrinkwrap.json to force a more recent > version > > > > of `inquirer` ourselves. Little work, no audit warnings for the > users. I > > > > could do that when the branch is ready. However, we should test the > whole > > > > thing with a alpha suffix or something like that first. > > > > > > > > Am Fr., 14. Sep. 2018 um 21:18 Uhr schrieb Chris Brody < > > > > chris.br...@gmail.com>: > > > > > > > >> Unfortunately I spotted a catch-22 situation while working on CLI > > > >> 8.1.x WIP in https://github.com/apache/cordova-cli/pull/326: > > > >> * insight@0.8 (0.8.4) has the audit issue > > > >> * newer insight starting with 0.9 uses inquirer@5 which does not > > > >> support Node.js 4. > > > >> > > > >> I can think of the following alternatives: > > > >> > > > >> 1. skip the proposed 8.1.0 minor release > > > >> 2. publish 8.1.0 minor release with known audit issue in the CLI > > > >> 3. drop use of insight in 8.1.0 minor release > > > >> 4. ask insight to publish 0.8.5 release that resolves the audit > issue > > > >> 5. publish special fork of insight which resolves the audit issue > for > > > >> 8.1.0 minor release > > > >> > > > >> Disadvantages of each alternative: > > > >> > > > >> 1: Users do not get some needed updates before the next major > release. > > > >> I think the major ones are: > > > >> - use of cordova-android@~7.1.x by default > > > >> - use of cordova-windows@~6.0.x by default > > > >> > > > >> 2: Bad practice, with possible responsibility for unknown security > > > >> issues. While I would not expect any real security issues in > practice, > > > >> I would say better safe than sorry. > > > >> > > > >> 3. I think this kind of behavior should not be dropped in minor > > > >> release, only to come back in next major release. > > > >> > > > >> 4. I highly doubt they would be motivated to do such a thing for us. > > > >> Support for deprecated Node.js 4 is not desired in other projects > > > >> unless absolutely necessary. > > > >> > > > >> 5. One more package for us to manage and maintain, on a temporary > basis > > > >> > > > >> To be honest I really wouldn't mind if we would just make the new > > > >> release to drop Node.js 4 support and abandon support for the > existing > > > >> package releases. > > > >> On Fri, Sep 14, 2018 at 9:25 AM <raphine...@gmail.com> wrote: > > > >> > > > > >> > Am Fr., 14. Sep. 2018 um 14:15 Uhr schrieb Chris Brody < > > > >> > chris.br...@gmail.com>: > > > >> > > > > >> > > Thanks Raphael for the reminder about insight, which I > overlooked. I > > > >> > > personally do not like the idea of an extra reminder message > before > > > the > > > >> > > next major release. I would like to consider this over the > weekend > > > >> > > > > > >> > > > > >> > That could be resolved in a few ways: > > > >> > > > > >> > - rolling back to previous version (can't remember if it had > audit > > > >> > issues) > > > >> > - Using insight's `config` option [1] with a config provider > that > > > uses > > > >> > the same file as before. The commit that changed the config > store > > > was > > > >> [2] > > > >> > > > > >> > Cheers > > > >> > > > > >> > [1]: https://github.com/yeoman/insight#config > > > >> > [2]: > > > >> > > > > >> > > > > https://github.com/yeoman/insight/commit/dae6dd4b73b9cebe3c1ad877f467b7b1c58c1d4c > > > >> > > > >> > --------------------------------------------------------------------- > > > >> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org > > > >> For additional commands, e-mail: dev-h...@cordova.apache.org > > > >> > > > >> > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org > > > For additional commands, e-mail: dev-h...@cordova.apache.org > > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org > For additional commands, e-mail: dev-h...@cordova.apache.org > >
