Skip to content

Conversation

@bhearsum
Copy link
Contributor

I might be missing something, but I'm not sure how the code is supposed to work in its current form - when I use debug logging, data quite clearly does not contain hashes, and the block above this pretty clear is meant to built up the hashes that the caller wants.

@codecov
Copy link

codecov bot commented Sep 10, 2020

Codecov Report

Merging #392 into master will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #392   +/-   ##
=======================================
  Coverage   94.20%   94.20%           
=======================================
  Files          11       11           
  Lines        1157     1157           
=======================================
  Hits         1090     1090           
  Misses         67       67           
Impacted Files Coverage Δ
pyup/requirements.py 88.46% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6fb8638...d301812. Read the comment docs.

@rafaelpivato rafaelpivato self-assigned this Dec 27, 2020
@rafaelpivato rafaelpivato added the considering Under consideration label Dec 27, 2020
Copy link
Contributor

@rafaelpivato rafaelpivato left a comment

Choose a reason for hiding this comment

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

You are totally right! The PyPi Warehouse JSON API return no hashes field and the function is clearly trying to return a list of hashes. I am wondering what other impacts with have with this.

Do you think you would be able to add a unit test for this? Maybe you mock some PyPi Warehouse API result and check its results. If you can do that, it would be great to avoid regressions.

https://warehouse.readthedocs.io/api-reference/json.html

@rafaelpivato rafaelpivato added bug and removed considering Under consideration labels Dec 27, 2020
@cblegare
Copy link

If you can do that, it would be great to avoid regressions.

Avoiding regression is great, but in the meantime, the tool is broken. Please consider merging this as-is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants