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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

packaged code to handle Pipfile.lock #2116

Merged
merged 6 commits into from Jul 22, 2020

Conversation

rpotter12
Copy link
Collaborator

Fixes #2082
Packagedcode to handle Pipfile.lock

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 馃搧

Copy link
Contributor

@steven-esser steven-esser left a comment

Choose a reason for hiding this comment

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

@rpotter12 Just one minor change left :)

Then this looks good to me.

src/packagedcode/pypi.py Outdated Show resolved Hide resolved
Copy link
Contributor

@steven-esser steven-esser left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@pombredanne Anything to add here?

Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

This is looking great and I just have a few minor nits.

src/packagedcode/pypi.py Outdated Show resolved Hide resolved
src/packagedcode/pypi.py Outdated Show resolved Hide resolved
src/packagedcode/pypi.py Outdated Show resolved Hide resolved
src/packagedcode/pypi.py Outdated Show resolved Hide resolved
if '_meta' in data:
for name, meta in data['_meta'].items():
if name=='hash':
sha256 = meta.get('sha256')
Copy link
Member

Choose a reason for hiding this comment

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

Do you know what is this sha256 about? A download archive? the original Pipfile?

Copy link
Contributor

Choose a reason for hiding this comment

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

@pombredanne This was originally brought up by me. It the the sha256 of the original pipfile (I think). Since it was in the data, I suggested we should collect it. We may not want to do this, as it is slightly confusing. Also, I am unsure if this particular hash will be present in most pipfiles.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pombredanne The sha256 which we are collecting in this packagedcode is of original pipfile. So I think we should detect this and add this in sha256 :)

Copy link
Member

Choose a reason for hiding this comment

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

ok, please enter a ticket to revisit this. I am not sure we can do much of anything with this as this is not the sha256 of some package archive of sorts?

"sha256:758cb50abddc03e4563fd9e7f03db56e3e87b58c0bd01247360326e5c0c7ffa5",
"sha256:0d7f6e959fe53f3960a23d73f35e1fce61348b30915b6664309ca756de7c1f89",
"sha256:d258b0a71994f7770599835249cece1caef3c70def868c4915e6e5ca49b67d15"
],
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we should add these checksums to the packagedcode model?

Copy link
Contributor

Choose a reason for hiding this comment

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

@pombredanne This was originally brought up by me. It the the sha256 of the original pipfile (I think). Since it was in the data, I suggested we should collect it. We may not want to do this, as it is slightly confusing. Also, I am unsure if this particular hash will be present in most pipfiles.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pombredanne The checksums for which you talking about is of dependencies and I know only little bit about checksums. I think if we don't add sha256 in our packagedcode model that will also be good.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe If we need to collect these, we can create a new ticket for this. :)

Signed-off-by: rpotter12 <rohitpotter12@gmail.com>
Signed-off-by: rpotter12 <rohitpotter12@gmail.com>
Signed-off-by: rpotter12 <rohitpotter12@gmail.com>
Signed-off-by: rpotter12 <rohitpotter12@gmail.com>
Signed-off-by: rpotter12 <rohitpotter12@gmail.com>
Signed-off-by: rpotter12 <rohitpotter12@gmail.com>
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@pombredanne pombredanne merged commit 8cd0f07 into nexB:develop Jul 22, 2020
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.

Parse Python(pipfile.lock) files
3 participants