snyk / nodejs-lockfile-parser

Generate a Snyk dependency tree from package-lock.json or yarn.lock file
Other
59 stars 28 forks source link

BREAKING CHANGE: remove tree size limit #94

Closed admons closed 3 years ago

admons commented 3 years ago

What this does

As we are creating optimised trees (see https://github.com/snyk/nodejs-lockfile-parser/pull/93), and that in most places we don't serialise the DepTree object, we don't need to really care about the size of the tree, but on the number of dependencies. After reviewing most of the "big trees" tickets in Zendesk, the number of dependencies are limited by thousands, which shouldn't be a problem any more.

This pr removes the limit of the tree size.

This is a breaking change, as we must verify that this plugin consumers doesn't serialise this object.

robcresswell commented 3 years ago

Can we break up PRs with separate concerns please? We shouldn't be changing CI config at the same time as config stuff. Makes rollbacks harder.

anthogez commented 3 years ago

In addition to what was already mentioned about separate prs. Would be nice to have the breaking change pr following this pattern

commit label: refactor: remove tree size limit

commit message body: (same content of your GH notes) BREAKING CHANGE: As we are creating optimised trees (see #93), and that in most places we don't serialise the DepTree object, we don't need to really care about the size of the tree, but on the number of dependencies. After reviewing most of the "big trees" tickets in Zendesk, the number of dependencies are limited by thousands, which shouldn't be a problem any more.

This pr removes the limit of the tree size. This is a breaking change, as we must verify that this plugin consumers doesn't serialise this object.

mhassan1 commented 3 years ago

This was a blocker for me. As a workaround, I was able to execute SNYK_YARN_TREE_SIZE_LIMIT=999999999999 snyk test to ignore the limit.