search

pfalcon / pycopy-lib

Standard library of the Pycopy project, minimalist and light-weight Python language implementation
https://github.com/pfalcon/pycopy
Other
246 stars 70 forks source link

os: adds .putenv() method #60

Closed MaxTurchin closed 3 years ago

MaxTurchin commented 3 years ago

Needed realpath and putenv functionality for my work. Though this might be of use :)

pfalcon commented 3 years ago

Adding these would be useful, thanks for looking into that!

The common recommendation is to open one PR per one feature. Let's concentrate of os.putenv() first. There're at least 2 issues: 1) What you implemented doesn't correspond to os.putenv() signature: https://docs.python.org/3/library/os.html#os.putenv; 2) please do "git log", and try to write your commit message as close as possible to what's already in git log.

Thanks!

MaxTurchin commented 3 years ago

Sure thing! I think my latest change does the trick. I'll open another PR for os.path.realpath ()

pfalcon commented 3 years ago

Thanks for splitting it and for updates.

os: Add .putenv() method

You picked just the right format for commit messages: package name, colon, then description with capital letter (I also put dot at the end, but don't nitpick others up to that ;-) ). Please check the realpath patches for that format.

Also, putenv() if the function, not method, so I updated the message after all.

pfalcon commented 3 years ago

Another useful thing to do is to add a test (if easily possible), or if not, a manual example a human can run and check that the function (still) works as expected. I added a kinda-test in https://github.com/pfalcon/pycopy-lib/blob/master/os/test_putenv.py.

pfalcon commented 3 years ago

Merged/released on PyPI, thanks!