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

Add default for SFTPClient.listdir #673

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Add default for SFTPClient.listdir #673

merged 1 commit into from
Jul 17, 2024

Conversation

tjstum
Copy link
Contributor

@tjstum tjstum commented Jul 17, 2024

At runtime, it's possible to call listdir() (i.e. without arguments) and get a Sequence[str] back, due to the default argument in the implementation. However, there is no @overload that contains 0 arguments, so mypy doesn't think that it's possible.

This adds the presence of the default to the sequence-of-strings returning overload, to match the implementation

(This is a resubmit of #671 but based on develop instead of master)

At runtime, it's possible to call `listdir()` (i.e. without arguments)
and get a `Sequence[str]` back, due to the default argument in the
implementation. However, there is no `@overload` that contains 0
arguments, so mypy doesn't think that it's possible.

This adds the presence of the default to the sequence-of-strings
returning overload, to match the implementation
Copy link

codecov bot commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.85%. Comparing base (fab96ac) to head (b07a48d).
Report is 28 commits behind head on develop.

Additional details and impacted files
@@            Coverage Diff             @@
##           develop     #673     +/-   ##
==========================================
  Coverage    99.85%   99.85%             
==========================================
  Files           95      101      +6     
  Lines        27784    29283   +1499     
  Branches      2908     3085    +177     
==========================================
+ Hits         27743    29242   +1499     
  Misses          38       38             
  Partials         3        3             

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

@ronf ronf merged commit 4b4aa73 into ronf:develop Jul 17, 2024
16 checks passed
@ronf
Copy link
Owner

ronf commented Jul 17, 2024

Thanks very much! This is now merged.

Also, thanks for pointing out the text about the branches in CONTRIBUTING.rst. In the early days of AsyncSSH, I did accept small fixes directly in the "master" branch, but it ended up making the branch history messier when it came time to move changes from "develop" to "master". I later switched to having "develop" be the default branch in GitHub and having all changes go in there first, but I never updated CONTRIBUTING.rst to reflect that change. This is now fixed.

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.

2 participants