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

Skip adding directories to RECORD with wheel tags #559

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

mwtoews
Copy link
Contributor

@mwtoews mwtoews commented Aug 18, 2023

Closes #558

Tests are not included yet, but advice would be welcome. The example with tests/test_tagopt.py would probably needed to be modified to add some dummy data files in a subdir. Some testing has been modified, but advice on more is welcome.

This PR also removes (now) unnecessary use of OrderedDict, and tidies a str concat.

@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05% 🎉

Comparison is base (2c779d9) 72.29% compared to head (2a200a3) 72.35%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #559      +/-   ##
==========================================
+ Coverage   72.29%   72.35%   +0.05%     
==========================================
  Files          13       13              
  Lines        1083     1085       +2     
==========================================
+ Hits          783      785       +2     
  Misses        300      300              
Files Changed Coverage Δ
src/wheel/cli/tags.py 89.33% <100.00%> (+0.29%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@henryiii
Copy link
Contributor

This only removed listing the directories, not the contents of directories, so looks good to me.

Personally, I recommend splitting unrelated cleanup & fixes/features if possible, makes review easier. But IMO the changes look good.

@mwtoews
Copy link
Contributor Author

mwtoews commented Aug 21, 2023

Personally, I recommend splitting unrelated cleanup & fixes/features if possible, makes review easier. But IMO the changes look good.

I normally do that too, so it's split to #561

@agronholm
Copy link
Contributor

agronholm commented Aug 21, 2023

Please add a note to the changelog (docs/news.rst) and credit yourself.

@agronholm agronholm merged commit 3856f7c into pypa:main Aug 21, 2023
17 checks passed
@agronholm
Copy link
Contributor

Thanks!

@mwtoews mwtoews deleted the fix-record-for-tags branch August 21, 2023 22:56
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.

wheel tags adds directories to RECORD causing issues with pip uninstall
3 participants