-
Notifications
You must be signed in to change notification settings - Fork 128
Conversation
287171f
to
b18f50a
Compare
I'll take free help + irc spam any time you have spare cycles ^_^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR 👍
The audit-filter binary is compiled for linux, so it won't run on mac. How hard would it be to add mac support?
The npm version required is a problem locally until #4804 is fixed. (Or we could document that as a requirement for the project). |
to make it valid JSON
istanbul middleware
https://www.npmjs.com/advisories/681 at paths: jpm > firefox-profile > adm-zip web-ext > firefox-profile > adm-zip These 1) run from build tools that already have access to local the local filesystem and 2) do not take input files outside the repo
0a1444d
to
e4ffef7
Compare
OK made some changes:
Haven't tested this PR on OSX (have tested audit-filer), but try running: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I'm not sure I'm qualified to review all the platform detection code in the shell script, but it looks fine at a glance, and does work on mac. I also tried removing one of the .nsprc entries, and the linter correctly failed on the missing advisory. Merging.
Assuming no new insecure deps, this should fix #4803 (for CI at least).
OK this is finally ready for review. Sorry about all the spam in #screenshots on irc.