Deprecate default of Git.show(strip_newline_in_stdout=False)#1948
Deprecate default of Git.show(strip_newline_in_stdout=False)#1948SonOfLilit wants to merge 3 commits intogitpython-developers:mainfrom
Conversation
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot for contributing.
I think with a few minor modifications this PR can be merged.
| if self.GIT_PYTHON_TRACE and (self.GIT_PYTHON_TRACE != "full" or as_process): | ||
| _logger.info(" ".join(redacted_command)) | ||
|
|
||
| if strip_newline_in_stdout and command[:2] == ["git", "show"]: |
There was a problem hiding this comment.
"git" shouldn't be hard-coded, and I am pretty sure there is a variable for this somewhere.
|
|
||
| if strip_newline_in_stdout and command[:2] == ["git", "show"]: | ||
| warnings.warn( | ||
| "Git.show() has strip_newline_in_stdout=True by default, which probably isn't what you want and will " |
There was a problem hiding this comment.
| "Git.show() has strip_newline_in_stdout=True by default, which probably isn't what you want and will " | |
| "Git.show() has strip_newline_in_stdout=True by default, which probably isn't what you want and may " |
| return r | ||
|
|
||
| @with_rw_directory | ||
| def test_warn_when_strip_newline_in_stdout(self, rw_dir): |
There was a problem hiding this comment.
Let's put the additional assertion done in test_do_not_strip_newline_in_stdout here so that it's clearer what's under test: The deprecation warning if git show is used and strip_newline_in_stdout=True. This way, test_do_not_strip_newline_in_stdout can be removed. I'd also hope that the assertion then can be changed to use the default value for strip_newline_in_stdout, which is probably True, the cause of the confusion in this case.
As per our discussion in #1377.
Also fix a few tests that were broken by my git being configured to create new repos with
mainas the initial branch, so that all my tests pass.