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

Get the package into a working state again #1705

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Get the package into a working state again #1705

wants to merge 18 commits into from

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Apr 7, 2022

While proper modernization seems due, this PR for now tries to get all the things working again.

  • Updates dependencies but doesn't go to lengths in regards of breaking dependency changes yet.
  • Continued work towards separation of the protobufjs and protobufjs-cli packages.
  • Removes dist/ files from source control.
  • Updates CI scripts, drops testing on Node.js < 12 and sets up a new automated publishing step.

@Qard
Copy link

Qard commented Apr 8, 2022

Can you do something about #1704 while you're in there?

@dcodeIO
Copy link
Member Author

dcodeIO commented Apr 9, 2022

I think this should be fixed by the dependencies update. The referenced PR doesn't do this I believe, since if package-lock is honored, there is no change as it hasn't been updated, and if it is not, the most recent ^7 will be installed anyway.

@Qard
Copy link

Qard commented Apr 10, 2022

Not updating the bottom end of a caret range in package.json means lower numbers are still considered valid. If npm tries to dedupe and flatten dependencies it may pick an older version to be in-range of all depending modules. Generally the package.json should be updated in the case of security vulnerabilities to explicitly block npm from allowing the module to depend on vulnerable versions of the dependency.

@dcodeIO
Copy link
Member Author

dcodeIO commented Apr 10, 2022

TIL, thanks :)

@craigmarker
Copy link

Anything we can do to help push this along? I'm interested in getting access to af1b449 which fixes an option parsing error I'm experiencing. I'd also be interested in providing a fix to parse EnumValueOptions: #1631

Thanks!

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.

4 participants