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.

Sign up for GitHub

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

Jump to bottom

Add Pipenv lockfile support (Resolves #71) #66

Merged
merged 6 commits into from
Jan 8, 2023
Merged

Add Pipenv lockfile support (Resolves #71) #66

merged 6 commits into from
Jan 8, 2023

Conversation

newdominic
Copy link
Contributor

@newdominic newdominic commented Dec 16, 2022 •
edited by inferno-chromium
Loading

Add support for Pipenv.lock. Fixes #71

@oliverchang
Copy link
Collaborator

Thank you very much for the contribution! CC @G-Rath as well who contributed most of the existing lockfile code.

@inferno-chromium inferno-chromium mentioned this pull request Dec 16, 2022
Closed
@G-Rath
Copy link
Collaborator

G-Rath commented Dec 16, 2022

@oliverchang @another-rex @newdominic would one of you mind adding "Resolves #71" to the PR description? (I've not the necessary permissions)

G-Rath
G-Rath approved these changes Dec 16, 2022
View reviewed changes
Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

LGTM!

@inferno-chromium inferno-chromium changed the title Add Pipenv lockfile support Add Pipenv lockfile support (Resolves #71) Dec 16, 2022
@inferno-chromium
Copy link
Contributor

@oliverchang @another-rex @newdominic would one of you mind adding "Resolves #71" to the PR description? (I've not the necessary permissions)

done! we should start thinking about adding more project contributors early next year. sorry for the hassle till then.

@G-Rath
Copy link
Collaborator

G-Rath commented Dec 16, 2022

@inferno-chromium thanks, though you've added it to the PR title when it needs to be in the description for it to count as a link😅

@inferno-chromium
Copy link
Contributor

@inferno-chromium thanks, though you've added it to the PR title when it needs to be in the description for it to count as a link😅

Done, forgot that.

another-rex
another-rex reviewed Dec 18, 2022
View reviewed changes
Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

Thank you! Can you also add a test case with a package that doesn't have a version? This can happen if a package is a local path for example.

e.g. This lockfile entry

pkg/lockfile/ecosystems_test.go Outdated Show resolved Hide resolved
@another-rex another-rex self-assigned this Dec 18, 2022
@newdominic
Copy link
Contributor Author

Thank you! Can you also add a test case with a package that doesn't have a version? This can happen if a package is a local path for example.

e.g. This lockfile entry

Done! Also the comment in pkg/lockfile/ecosystems_test.go has been modified.

oliverchang
oliverchang approved these changes Jan 4, 2023
View reviewed changes
Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

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

thank you very much!

Sorry for the delay in review -- most of our team was out over the holiday period.

@oliverchang
Copy link
Collaborator

/gcbrun

@oliverchang
Copy link
Collaborator

@newdominic Looks like there's a minor merge conflict. Would you mind fixing this up? I don't seem to be able to push to your fork branch to do this myself.

@another-rex
Copy link
Collaborator

/gcbrun

@another-rex
Copy link
Collaborator

LGTM, thank you!

@another-rex another-rex merged commit 3d74a72 into google:main Jan 8, 2023
@G-Rath G-Rath mentioned this pull request Jan 12, 2023
Merged
julieqiu pushed a commit to julieqiu/osv-scanner that referenced this pull request May 2, 2023
@newdominic @another-rex @julieqiu
Add Pipenv lockfile support (Resolves google#71) (google#66)
798c594 
* Add Pipenv lockfile support

* Skip when a package doesn't have a version

* Update pkg/lockfile/ecosystems_test.go

Co-authored-by: Rex P <106129829+another-rex@users.noreply.github.com>

* fix comment

Co-authored-by: Rex P <106129829+another-rex@users.noreply.github.com>
julieqiu pushed a commit to julieqiu/osv-scanner that referenced this pull request May 2, 2023
@newdominic @another-rex @julieqiu
Add Pipenv lockfile support (Resolves google#71) (google#66)
cff6478 
* Add Pipenv lockfile support

* Skip when a package doesn't have a version

* Update pkg/lockfile/ecosystems_test.go

Co-authored-by: Rex P <106129829+another-rex@users.noreply.github.com>

* fix comment

Co-authored-by: Rex P <106129829+another-rex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@another-rex another-rex another-rex approved these changes

@G-Rath G-Rath G-Rath approved these changes

@oliverchang oliverchang oliverchang approved these changes

Assignees

@another-rex another-rex

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for pipenv lock files
5 participants
@newdominic @oliverchang @G-Rath @inferno-chromium @another-rex