[rush] Properly handle Git worktrees - #994
Conversation
| } | ||
| } catch (ex) { | ||
| // ignore | ||
| } |
There was a problem hiding this comment.
What kind of exceptions are we expecting to catch and ignore here?
If this throws and catches an exception as part of a normal expected state that is not an error (an "unexceptional exception"), is there a test we can add that would avoid that? (Unexceptional exceptions make debugging harder, because when you try to use the "break on exceptions" feature it constantly stops at exceptions that don't reflect any actual problem.)
There was a problem hiding this comment.
Added a comment and changed the try/catch to wrap only the most relevant part (which actually is very unlikely to throw)
| let branch: string | undefined = undefined; | ||
| try { | ||
| branch = gitInfo().branch; | ||
| branch = Git.getGitInfo()!.branch; |
There was a problem hiding this comment.
!.branch [](start = 31, length = 8)
How do we know that getGitInfo() doesn't return undefined here?
There was a problem hiding this comment.
It could, but the try/catch would catch it.
There was a problem hiding this comment.
Actually I'll remove the try/catch and do an if instead
There was a problem hiding this comment.
Agreed. A null reference exception is always a program malfunction. We would never intentionally catch this sort of exception.
In reply to: 241253975 [](ancestors = 241253975)
When copying Git hooks in a Git working directory which is a worktree, the destination should be under the Git metadata directory for the current worktree. Copying into
path.join(repoInfo.worktreeGitDir, 'hooks')(repoInfois returned bygit-repo-info) will give the correct results for both worktrees and normal working directories.For example, if my main copy of a Git repo is at
D:\git\fabric-reactand the worktree I'm building in is atD:\git\fabric-react2, the hooks should get copied intoD:\git\fabric-react\.git\worktrees\fabric-react2\hooks(notD:\git\fabric-react2\.git\hooksas it was previously attempting).Updating
git-repo-infoto a new version which supports worktrees also makes Git email policy work in worktrees. I added typings to the latest git-repo-info version too, so the custom typings can be removed.(Fixes #990)