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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix modules.py def of 'cattrs' known 3rd party module #69

Merged
merged 1 commit into from
Dec 12, 2022
Merged

Fix modules.py def of 'cattrs' known 3rd party module #69

merged 1 commit into from
Dec 12, 2022

Conversation

whardier
Copy link
Contributor

This was missing one of the namespaces included in that library.

Got to deep dive into this project. Neat setup. This was a confusing headscratcher though. I'll be adding a TON to this known 3rd parties soon.

This was missing one of the namespaces included in that library.
@codecov
Copy link

codecov bot commented Dec 12, 2022

Codecov Report

Base: 93.63% // Head: 93.63% // No change to project coverage 👍

Coverage data is based on head (50448a0) compared to base (c5345a0).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #69   +/-   ##
=======================================
  Coverage   93.63%   93.63%           
=======================================
  Files           3        3           
  Lines         393      393           
=======================================
  Hits          368      368           
  Misses         25       25           
Impacted Files Coverage Δ
src/flake8_requirements/modules.py 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@arkq arkq merged commit a246dd4 into arkq:master Dec 12, 2022
@arkq
Copy link
Owner

arkq commented Dec 12, 2022

Thx!

I'll be adding a TON to this known 3rd parties soon.

OK, hope that you will not add every PyPi project :D

@whardier
Copy link
Contributor Author

Hah no. I maintain a standard set of packages that all "install" together.. very broad. I have cached information that can be used to build up the list where namespaces might be overlooked since this list hits a lot of areas.

However.. was there a staged installation proposed for this project ever? Install everything in requirements and inspect dist-info/RECORDS ? It might be more worthwhile to cache all those files here and scan them and allow the cache to grow. IDK.

@arkq
Copy link
Owner

arkq commented Dec 12, 2022

was there a staged installation proposed for this project ever?

No, thing like that was never proposed. However, I'm not sure whether I understand you correctly with staged installation in regard to requirements checking... The case for this checker is to use it everywhere in a consistent way. So, locally growing cache might indeed provide some valuable info, but this will be of no use for CI static analysis where projects are not installed at all - required packages will not be collected beforehand in such setup.

@whardier
Copy link
Contributor Author

Understood. So then why not put a large portion of pypi in here if it is needed due to name mismatches and extra namespaces?

@arkq
Copy link
Owner

arkq commented Dec 12, 2022

So then why not put a large portion of pypi in here if it is needed due to name mismatches and extra namespaces?

Packages from PyPi are put here bit by bit, but this process is managed by community (at least that's how I see it). Projects which I am/was using are added by me, but the rest has to be done by contributors :)

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.

None yet

2 participants