Cody Ignore: remove the Git extension shim #4115
Conversation
valerybugakov
force-pushed
the
vb/git-extension-shim-removal
branch
from
0d7802f
to
c99e204
Compare
valerybugakov
force-pushed
the
vb/git-extension-shim-removal
branch
from
c99e204
to
1b9f8b2
Compare
valerybugakov
force-pushed
the
vb/git-extension-shim-removal
branch
from
76245b7
to
2da8d02
Compare
| @@ -0,0 +1,105 @@ | |||
| import ini from 'ini' | |||
There was a problem hiding this comment.
No functional changes. Only moved tree-walk functions in a separate file.
| import { gitRemoteUrlsFromTreeWalk } from './remote-urls-from-tree-walk' | ||
| import { mockFsCalls } from './test-helpers' | ||
|
|
||
| describe('gitRemoteUrlFromTreeWalk', () => { |
There was a problem hiding this comment.
No changes. Only moved to a new file.
There was a problem hiding this comment.
LGTM this seems like a good improvement in the fallback case. I’m still not sold this is a good long term solution. The long term plan with the git extension shim was to implement it with the client. IntelliJ has a git plugin with an API that we should be using for production settings in XXL repos. For example, at Twitter we required engineers to use a fork of git that was auto-configured in everybody’s IntelliJ setup. Given the focus of Cody Ignore is Enterprise customers, then we need to make sure our solutions work even for specialized repo setups.
|
I left a comment in the linked issue, but will repeat here that we should not try to implement our own file watchers. File watching is very difficult to get right in XXL repos and we should rely instead on the IDEs built in file watching APIs. |
|
One option is to keep the existing got extension shim but use the “disabled” option from IntelliJ until we allow the client to implement the necessary parts of the git extension. It might not require a lot of work to pull out the information from IntelliJ, and then we don’t have to revert the removals in this PR. |
| /** | ||
| * Reads the .git directory or file to determine the path to the git config. | ||
| */ | ||
| async function resolveGitConfigUri(uri: vscode.Uri): Promise<vscode.Uri | undefined> { |
There was a problem hiding this comment.
I think it's good to have this as a fallback solution when nothing else works (git extension is disabled, client doesn't git, ..) but I think it's a slippery slope to reimplement core features of git. My experience is that big companies end up using advanced git features that we may not be handling here. Also, this doesn't work for non-git systems like Perforce (which we support for several important customers).
We should really lean on using the higher-level version-control APIs that the IDEs provide (IntelliJ's plugin is excellent, and a ton of work has gone into making it scale for even the most difficult repos)
There was a problem hiding this comment.
One option is to keep the existing got extension shim but use the “disabled” option from IntelliJ until we allow the client to implement the necessary parts of the git extension.
I like it. We can keep only the "disabled" option until the integration is implemented.
We should really lean on using the higher-level version-control APIs that the IDEs provide
Agree with a hight level idea of leaning more towards the IDE provided functionality. We can figure out how to integrate it with the Git extension shim, when the time comes to prioritize it.
There was a problem hiding this comment.
- Created a follow-up Agent: allow the client to implement the necessary parts of the git extension #4165.
- Reverted the git extension shim removal.
- Changed the protocol to
git?: 'none' | 'enabled'to the need to refactor other clients. - Added logic to throw errors if the git extension is used in the agent with a linked issue.
gitcapability has been disabled.cody/vscode/src/chat/chat-view/SimpleChatPanelProvider.ts
Line 423 in e01888c
createFileSystemWatchershim #4136vscodeGitAPI.repositories#4137vscodeGitAPI.onDidOpenRepository#4138Test plan
CI