Title: Pre-deprecate setting Git.USE_SHELL by EliahKagan · Pull Request #1782 · gitpython-developers/GitPython · GitHub
Open Graph Title: Pre-deprecate setting Git.USE_SHELL by EliahKagan · Pull Request #1782 · gitpython-developers/GitPython
X Title: Pre-deprecate setting Git.USE_SHELL by EliahKagan · Pull Request #1782 · gitpython-developers/GitPython
Description: Fixes #1781 This does two things: Converts some comments on constants and other attributes to docstrings of the kind discussed in #1734, throughout the codebase (not limited to the git.cmd module). Rewrites the comment, now in the form of a docstring, for Git.USE_SHELL, to discourage setting it to True, briefly explain why that can be unsafe, and explain the Windows console suppression use case for which it had previously been recommended and that it is no longer necessary or useful for that case (#1781). I have held off on calling it "deprecated," which I think is a decision that could be made later. There may be ways to limit the downsides of shell=True that arise with dynamic methods that call Git.execute (including those that happen indirectly through other GitPython code). If so, and a change that improves the safety of most existing client code that uses shell=True or Git.USE_SHELL = True would be a breaking change, then that would be a further reason to officially deprecate setting Git.USE_SHELL to True. If so, but it could be done in a way that is not a breaking change, then that might be a reason not to deprecate it. In any case, issuing a DeprecationWarning when the attribute is set on the class will probably require a metaclass or new custom descriptor to be written, which is more complicated than changing the message and may benefit from being reviewed separately. The reason I've converted attribute/constant comments to non-reified docstrings and "pre-deprecated" USE_SHELL together in one pull request, rather than separately, is: Turning them into something that editors such as VS Code recognize and render as docstrings makes it so the revised "docstring" on USE_SHELL is more salient, increasing the benefit of the change. Making the changes separately, in the event that a release for an unrelated reason were to end up being done between them, would--depending on the order--either forgo some of the benefit, or risk the opposite effect of making the old and currently incorrect message more salient. However, I would be pleased to sever this into two separate PRs or otherwise change this. My commit message for 106bbe6 notes some of the above-mentioned considerations that may affect deprecation. If you'd prefer those thoughts not go in a commit message, I'd be pleased to amend it (or, if you prefer, I think GitHub will let you force push to my branch for the PR).
Open Graph Description: Fixes #1781 This does two things: Converts some comments on constants and other attributes to docstrings of the kind discussed in #1734, throughout the codebase (not limited to the git.cmd module)...
X Description: Fixes #1781 This does two things: Converts some comments on constants and other attributes to docstrings of the kind discussed in #1734, throughout the codebase (not limited to the git.cmd module)...
Opengraph URL: https://github.com/gitpython-developers/GitPython/pull/1782
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/checks(.:format) |
| route-controller | pull_requests |
| route-action | checks |
| fetch-nonce | v2:d8bb138c-0d64-3626-c678-d47f74bc4f10 |
| current-catalog-service-hash | 87dc3bc62d9b466312751bfd5f889726f4f1337bdff4e8be7da7c93d6c00a25a |
| request-id | E870:301010:7900D2:A68B53:6968AFA6 |
| html-safe-nonce | 6a2ff8c027251656b8e1a71254ff8527389a8a0c085378e627567f5c331cfd92 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJFODcwOjMwMTAxMDo3OTAwRDI6QTY4QjUzOjY5NjhBRkE2IiwidmlzaXRvcl9pZCI6IjI4ODQzODcxMDU4OTE5ODczNjYiLCJyZWdpb25fZWRnZSI6ImlhZCIsInJlZ2lvbl9yZW5kZXIiOiJpYWQifQ== |
| visitor-hmac | 54b4dcd2d39ee05727b88ada727500c92412835e571673811b1c34bd7ebd9670 |
| hovercard-subject-tag | pull_request:1655701183 |
| 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/1782/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 | Fixes #1781 This does two things: Converts some comments on constants and other attributes to docstrings of the kind discussed in #1734, throughout the codebase (not limited to the git.cmd module)... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | fdc7c66bd36a6c12eb8e771e806db863266e573fc299e77f27505a768d4f8a98 |
| 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 | 3223a6503d318917691422cdadfbe16cd8fb21e5 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width