Skip to content

Move os.fspath to _os #966

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

Merged
merged 3 commits into from
May 19, 2019
Merged

Move os.fspath to _os #966

merged 3 commits into from
May 19, 2019

Conversation

palaviv
Copy link
Contributor

@palaviv palaviv commented May 11, 2019

For #958.

@codecov-io
Copy link

Codecov Report

Merging #966 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #966      +/-   ##
==========================================
+ Coverage   64.87%   64.89%   +0.01%     
==========================================
  Files          90       90              
  Lines       16046    16055       +9     
  Branches     3634     3636       +2     
==========================================
+ Hits        10410    10419       +9     
  Misses       3231     3231              
  Partials     2405     2405
Impacted Files Coverage Δ
vm/src/stdlib/os.rs 70.14% <100%> (+1.03%) ⬆️

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 c276357...d96d04f. Read the comment docs.

@@ -133,34 +133,3 @@ def getenv(key, default=None):
The optional second argument can specify an alternate default.
key, default and the result are str."""
return environ.get(key, default)


def fspath(path):
Copy link
Member

Choose a reason for hiding this comment

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

Will keeping this funciton as _fspath just as cpython help to test new os_fspath works correctly?
Probably we can test 2 functions return same value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are testing against CPython function

@coolreader18
Copy link
Member

@palaviv I tried to merge master into your branch and I messed it up; would you mind adding use crate::obj::objtype; to the top of os.rs? I'll merge then.

@coolreader18 coolreader18 merged commit 82e424a into RustPython:master May 19, 2019
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.

4 participants