Add warning for git version parse failures #10446
Conversation
| @@ -94,6 +94,7 @@ def get_git_version(self) -> Tuple[int, ...]: | |||
| version = self.run_command(["version"], show_stdout=False, stdout_only=True) | |||
| match = GIT_VERSION_REGEX.match(version) | |||
| if not match: | |||
| logger.warning("Unable to parse '%s'.", version) | |||
There was a problem hiding this comment.
It may also be worth while to explain where we got that unparsable string, i.e.
| logger.warning("Unable to parse '%s'.", version) | |
| logger.warning("Unable to parse 'git version' output: %r.", version) |
There was a problem hiding this comment.
Do you mean something like the below?
"""
Equivalent of running 'git --version'
See run_command in parent class
"""
There was a problem hiding this comment.
I don’t quite understand what you meant. What I was trying to express is, this Unable to parse message will be seen by the user in the console, and when that happens the user may not fully understand the context of the message (why does the string fail to parse? Where does that string come from?) Therefore, I recommended adding git version output to the message, so the user understands the string is the output to git version and react accordingly (and yes, git --version gives the exact same output, but we are using git version right now so that should be used instead).
There was a problem hiding this comment.
Ah okay. I misunderstood, my apologies.
EDIT: What I can I do now to get this moving along?
github-actions
bot
added
the
needs rebase or merge
|
Just tagging #10361 so that people looking for "good first issue" know there's a PR open. |
|
(I updated the top post to include the “close” command so GitHub shows the PR more prominently in the issue.) |
There was a problem hiding this comment.
Hello @HausCloud. This is a really good PR!
Could you please rebase the branch, so we can keep going ahead?
|
Closing this out, due to lack of a response. |
This is what I came up with from studying the issue. Any guidance on how to improve this would be much appreciated. Thanks for your time.
Fix #10361.