Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

housekeeping: bump argparse dependency to ^1.0.10 #349

Merged
merged 3 commits into from
Jul 21, 2019
Merged

housekeeping: bump argparse dependency to ^1.0.10 #349

merged 3 commits into from
Jul 21, 2019

Conversation

shockey
Copy link
Collaborator

@shockey shockey commented Jul 21, 2019

Fixes #310; concurrent with PRs #312, #323, #327.

I started on a fresh branch to keep things simple.

  • Adds a simple unit test for the CLI.
  • Introduces a package-lock.json for contributors, in line with best practices for libraries.
    • If there's dissent on this, let's just drop the lockfile here and hash it out in another PR.
  • Existing local and CI tests look good.
  • Manual testing of the Remarkable binary and the specsplit support script reveal no breaking changes.

npm audit is now showing a clean slate for production dependencies:

➜ npm audit --json | jq '.advisories | map(.findings) | flatten | map(select(.dev == false))'
[]

This PR is clean and narrow only because of @TrySound's efforts around getting CI working again. Hat tip to @DarrenMack-OD who put a lot of effort into #323, which would've achieved the same end.


Per @jonschlinkert:

argparse should have released a patch to fix this. An issue should be created with that library, it shouldn't require a bump here.

Further, let's acknowledge that npm audit is lighting up Remarkable for something that isn't even a serious vulnerability to the library in the first place.

All that said... this has been a bother since February so let's just put it to bed 😄

@coveralls
Copy link

coveralls commented Jul 21, 2019

Coverage Status

Coverage decreased (-0.6%) to 98.584% when pulling 27a1b4d on bug/310 into 4240559 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.139% when pulling 76df13b on bug/310 into 4240559 on master.

@shockey shockey requested a review from jonschlinkert July 21, 2019 01:34
@TrySound
Copy link
Collaborator

Hi, I already started using yarn in this project. Could you please remove package-lock and use yarn instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

security vulnerability with using old version of underscore.string
3 participants