Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork966
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| "Git.show() has strip_newline_in_stdout=True by default, which probably isn't what you want andwill " | |
| "Git.show() has strip_newline_in_stdout=True by default, which probably isn't what you want andmay " |
| return r | ||
| @with_rw_directory | ||
| def test_warn_when_strip_newline_in_stdout(self, rw_dir): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Let's put the additional assertion done intest_do_not_strip_newline_in_stdout here so that it's clearer what's under test: The deprecation warning ifgit show is used andstrip_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 forstrip_newline_in_stdout, which is probablyTrue, 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.