Title: Better document env_case test/fixture and cwd by EliahKagan · Pull Request #1657 · gitpython-developers/GitPython · GitHub
Open Graph Title: Better document env_case test/fixture and cwd by EliahKagan · Pull Request #1657 · gitpython-developers/GitPython
X Title: Better document env_case test/fixture and cwd by EliahKagan · Pull Request #1657 · gitpython-developers/GitPython
Description: This improves how some of the changes in #1650 are documented, expanding comments and a docstring. I ended up doing this in four commits, but really it would probably be just as clear with two. If you'd like me to rebase this to have just two commits, please let me know. Identifying the steps in the "env_case" test This is my main motivation for opening this PR, and most of the change. I was thinking about how it was not immediately obvious why test_it_avoids_upcasing_unrelated_environment_variable_names (the "env_case" test) works and why it is done in this way. To improve on this, I wrote comments that divide it into four steps. Two of the steps occur in the test case method, and the others occur in the supporting env_case.py fixture. The comments are in both files. This only adds and changes comments, but just in case I have locally tested that it does not stop the test from passing, nor from failing when using the version of the code in git/ from just before the merge of #1650. Non-reentrancy of some context manager objects I had intended my brief mention of non-reentrancy to distinguish git.util.cwd from contextlib.chdir, which is fully reentrant by having each returned object maintain a stack of directories (but which is not available before Python 3.11). But this was not clear at all--I had nowhere mentioned contextlib.chdir!--and it risked giving the wrong impression that cwd couldn't be called multiple times in nested with-statements. Furthermore, the newly introduced patch_env function is non-reentrant in exactly the same narrow sense, but because it did not mention this and cwd did, this gave the wrong impression that patch_env was reentrant in this way. (I think patch_env actually doesn't need to document this--really cwd's limitation compared to contextlib.chdir is the only thing important enough to mention.) I've somewhat clarified this, but maybe further improvements can be made in the future. This can be make clearer with extended example, but I would be very reluctant to bloat the cwd docstring with fully worked examples. If anything, maybe there is a way to say it shorter. Or maybe the reentrancy information should be removed altogether.
Open Graph Description: This improves how some of the changes in #1650 are documented, expanding comments and a docstring. I ended up doing this in four commits, but really it would probably be just as clear with two. If ...
X Description: This improves how some of the changes in #1650 are documented, expanding comments and a docstring. I ended up doing this in four commits, but really it would probably be just as clear with two. If ...
Opengraph URL: https://github.com/gitpython-developers/GitPython/pull/1657
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/checks(.:format) |
| route-controller | pull_requests |
| route-action | checks |
| fetch-nonce | v2:924887bc-2789-8901-606c-d86c16032353 |
| current-catalog-service-hash | 87dc3bc62d9b466312751bfd5f889726f4f1337bdff4e8be7da7c93d6c00a25a |
| request-id | CFEE:30EB40:25C2E:33B0D:69693BF4 |
| html-safe-nonce | 571fe8af01320020507134df656ae82da6e1241f5c0ba6f82b1e35d346b96406 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJDRkVFOjMwRUI0MDoyNUMyRTozM0IwRDo2OTY5M0JGNCIsInZpc2l0b3JfaWQiOiI0NTA5MTM2NjEwNzgyNjkwMjkzIiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | 02e0d71127af6323a79473bd3e449da5b5f4719fad824ed59f972b7272ececea |
| hovercard-subject-tag | pull_request:1511366203 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,checks,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/gitpython-developers/GitPython/pull/1657/checks |
| twitter:image | https://avatars.githubusercontent.com/u/1771172?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/1771172?s=400&v=4 |
| og:image:alt | This improves how some of the changes in #1650 are documented, expanding comments and a docstring. I ended up doing this in four commits, but really it would probably be just as clear with two. If ... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 54182691a21263b584d2e600b758e081b0ff1d10ffc0d2eefa51cf754b43b51d |
| turbo-cache-control | no-preview |
| go-import | github.com/gitpython-developers/GitPython git https://github.com/gitpython-developers/GitPython.git |
| octolytics-dimension-user_id | 503709 |
| octolytics-dimension-user_login | gitpython-developers |
| octolytics-dimension-repository_id | 1126087 |
| octolytics-dimension-repository_nwo | gitpython-developers/GitPython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 1126087 |
| octolytics-dimension-repository_network_root_nwo | gitpython-developers/GitPython |
| turbo-body-classes | logged-out env-production page-responsive full-width full-width-p-0 |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | d69ac0477df0f87da03b8b06cebd187012d7a930 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width