search

microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.21k stars 12.51k forks source link

Revert "Add CopilotRelated command (#59963)" #60486

Closed sandersn closed 1 week ago

sandersn commented 1 week ago

This reverts commit 52c59dbcbee274e523ef39e6c8be1bd5e110c2f1. This feature didn't help, but it took a long time to verify that.

typescript-bot commented 1 week ago

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, @zkat, and @joj for you. Feel free to loop in other consumers/maintainers if necessary.

typescript-bot commented 1 week ago

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

sandersn commented 1 week ago

@RyanCavanaugh both @DanielRosenwasser and @sheetalkamat are out right now, so I want to make sure you know about this. It'll need a cherry-pick to 5.7-rc as well.

sandersn commented 1 week ago

Do all clients understand not to attempt to call this anymore, given the message is removed?

No. I thought I had some additional check for the existence of copilotRelated, but I just wrote the usual version check. So I think your offline suggestion of turning this into an empty-array-returning stub until 5.8 is a good one. That way vscode 1.95 won't crash on 5.7.

Actually, it's probably easier to write that from current main instead of on top of a revert, so I'll close this PR and open a new one.

sandersn commented 1 week ago

Superceded by #60488