Skip to content

Update package scan for package.json file - #2457

Merged
pombredanne merged 4 commits into
aboutcode-org:developfrom
Divyansh2512:fixes
Mar 26, 2021
Merged

pombredanne merged 4 commits into
aboutcode-org:developfrom
Divyansh2512:fixes

Conversation

@Divyansh2512

@Divyansh2512 Divyansh2512 commented Mar 25, 2021

Copy link
Copy Markdown
Contributor

Fixes #2382

Checks for existance of name constraint only instead of both name and version.

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁

Signed-off-by: Divyansh2512 <2512sharmadivyansh@gmail.com>
Signed-off-by: Divyansh2512 <2512sharmadivyansh@gmail.com>
Signed-off-by: Divyansh2512 <2512sharmadivyansh@gmail.com>
Signed-off-by: Divyansh2512 <2512sharmadivyansh@gmail.com>
@Divyansh2512

Copy link
Copy Markdown
Contributor Author

@MaJuRG @pombredanne Kindly review!

@steven-esser steven-esser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Divyansh2512 Thanks for this!

@pombredanne anything else to add here?

@rpotter12 rpotter12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@pombredanne Please review the PR.

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great! I have a nitpicking comment but that's not in the way of merging.

try:
npm.parse(test_file)
except AttributeError as e:
assert "'NoneType' object has no attribute 'to_dict'" in str(e)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there be a better exception raises, like a new one such as a NpmWithoutNameError ?
But we can merge this as is for now. An update later is welcomed!

@pombredanne
pombredanne merged commit 96c73a2 into aboutcode-org:develop Mar 26, 2021
@pombredanne

Copy link
Copy Markdown
Member

@rpotter12 @MaJuRG thanks for the review too BTW 🙇

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.

Cannot collect package scan from an npm package.json without a version aka. "private"

4 participants