snyk / nodejs-lockfile-parser

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

fix: correct messaging in out of sync errors if using yarn2 #109

Closed JamesPatrickGill closed 3 years ago

JamesPatrickGill commented 3 years ago

What this does

Getting an out of sync error when using yarn2 now produces accurate messaging instead of undefined being present

admons commented 3 years ago

Hey @jan-stehlik , I don't think that this PR needs additional tests In general, I don't think we should test "configurations" - either json files or constants that acts as factories If you think otherwise, let's talk :)

snyksec commented 3 years ago

:tada: This PR is included in version 1.34.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

jan-stehlik commented 3 years ago

Hey @jan-stehlik , I don't think that this PR needs additional tests In general, I don't think we should test "configurations" - either json files or constants that acts as factories If you think otherwise, let's talk :)

hi @admons thanks for reaching out. Would you say this is a config change or error handling change? I probably should have elaborated on what i mean, i wasn't referring to test - make sure LOCK_FILE_NAME includes yarn2 key. That wouldn't create any real value imo and i agree not to create such tests.

I was referring to a test more like - given we have yarn 2 project (arrange), something fails (act) we display human friendly error message to a user (assert).

I definitely see value in testing both happy and unhappy paths (error handling) as I would like to get automated confidence for both. What do you think?