Skip to content

Add Pipfile Analyzer#2877

Merged
jeremylong merged 2 commits intodependency-check:mainfrom
fcano:add-pipfile-support
Oct 12, 2020
Merged

Add Pipfile Analyzer#2877
jeremylong merged 2 commits intodependency-check:mainfrom
fcano:add-pipfile-support

Conversation

@fcano
Copy link
Copy Markdown
Contributor

@fcano fcano commented Oct 9, 2020

Description of Change

Add a basic Pipfile Analyzer.

Have test cases been added to cover the new functionality?

yes

@boring-cyborg boring-cyborg Bot added ant changes to ant cli changes to the cli core changes to core documentation site documentation maven changes to the maven plugin tests test cases utils changes to utils labels Oct 9, 2020
jeremylong
jeremylong requested changes Oct 10, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jeremylong jeremylong 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 for such a complete PR! Just one minor suggestions - unless you can think of any other way to obtain vendor information from pip...

Comment thread core/src/main/java/org/owasp/dependencycheck/analyzer/PipfileAnalyzer.java
@fcano
Copy link
Copy Markdown
Contributor Author

fcano commented Oct 11, 2020

@jeremylong do you know why the test failed? Looking into the logs it does not seem a problem with the commit.

@jeremylong jeremylong added this to the 6.0.3 milestone Oct 11, 2020
@jeremylong
Copy link
Copy Markdown
Collaborator

jeremylong commented Oct 11, 2020

Apparently I broke the build. If you git rebase main this should resolve itself. Otherwise I know this builds and is correct - I can just merge...

@fcano
Copy link
Copy Markdown
Contributor Author

fcano commented Oct 11, 2020 •
edited
Loading

The idea is to 'git rebase main' the branch 'add-pipfile-support' and drop the last commit (ab94853)?

@fcano fcano force-pushed the add-pipfile-support branch from ab94853 to 7533e9a Compare October 11, 2020 18:27
@fcano
Copy link
Copy Markdown
Contributor Author

fcano commented Oct 11, 2020

I think I did it. Now, the PR does not have an evidence for the VENDOR. Is it mandatory? I don't see that line in the PipAnalyzer for example. It only has these two lines:

https://github.com/jeremylong/DependencyCheck/pull/2526/files#diff-13b45a953258e6e7d4327bc57ef26169R163-R164

jeremylong
jeremylong reviewed Oct 11, 2020
View reviewed changes
Comment thread core/src/main/java/org/owasp/dependencycheck/analyzer/PipfileAnalyzer.java
@jeremylong
Copy link
Copy Markdown
Collaborator

jeremylong commented Oct 11, 2020

I just put the line adding the vendor evidence back. As to what happened - you likely accepted my change request in the github UI and did not pull the update back in before rebasing.

Seriously - that you for such a complete PR!

@fcano
Copy link
Copy Markdown
Contributor Author

fcano commented Oct 12, 2020

I'm sorry because I don't have a lot of experience with git and I got lost with the rebase. Now I see your commit in the pull request and the tests passing. Is everything ok now? Is there anything else that I have to do?

@jeremylong
Copy link
Copy Markdown
Collaborator

jeremylong commented Oct 12, 2020

Yup - everything is good. Thanks for the PR!

@jeremylong jeremylong merged commit 67ce215 into dependency-check:main Oct 12, 2020
@fcano fcano deleted the add-pipfile-support branch October 13, 2020 05:49
This was referenced Mar 15, 2021
Closed
Closed
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jan 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jeremylong jeremylong jeremylong approved these changes

Assignees

No one assigned

Labels

ant changes to ant cli changes to the cli core changes to core documentation site documentation maven changes to the maven plugin tests test cases utils changes to utils

Projects

None yet

Milestone

6.0.3

Development

Successfully merging this pull request may close these issues.

2 participants

@fcano @jeremylong