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

Remove unnecessary dependency on six. #743

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

charettes
Copy link
Contributor

@charettes charettes commented Jul 15, 2023

Remove the last remaining usage of it in VCR.testcase.

@charettes charettes marked this pull request as ready for review July 15, 2023 17:20
@codecov-commenter
Copy link

codecov-commenter commented Jul 15, 2023

Codecov Report

Merging #743 (3ff822d) into master (7e11cfc) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #743      +/-   ##
==========================================
- Coverage   90.63%   90.52%   -0.12%     
==========================================
  Files          28       28              
  Lines        1794     1794              
  Branches      322      322              
==========================================
- Hits         1626     1624       -2     
- Misses        133      135       +2     
  Partials       35       35              
Impacted Files Coverage Δ
vcr/config.py 95.06% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@hartwork hartwork left a comment

Choose a reason for hiding this comment

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

@charettes thanks for the pull request 👍, the output I see from running pytest -v tests/unit/test_vcr.py::TestVCRClass is unchanged, very nice. There is one tiny formatting issue that running black . would fix and that currently causes the red CI at https://github.com/kevin1024/vcrpy/actions/runs/5563344118/jobs/10164187675?pr=743#step:5:37 . Could you apply the tiny fix this needs for a green CI?

@jairhenrique do you have any concerns about the approach taken by this pull request?

Remove the last remaining usag of it in VCR.testcase.
@charettes
Copy link
Contributor Author

@hartwork sure thing, thanks for the review! Just forced-pushed a commit that addresses the formatting issue.

Copy link
Collaborator

@hartwork hartwork left a comment

Choose a reason for hiding this comment

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

@hartwork sure thing, thanks for the review! Just forced-pushed a commit that addresses the formatting issue.

@charettes perfect, thank you! 👍

@hartwork hartwork merged commit 1d979b0 into kevin1024:master Jul 18, 2023
6 checks passed
@charettes charettes deleted the remove-six branch July 19, 2023 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants