-
Notifications
You must be signed in to change notification settings - Fork 61
Remove proxy-cached feature flag #1600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: pavera <660677+pavera@users.noreply.github.com>
|
@copilot also do not set the PROXY_CACHE env var when running the proxy container. The proxy is also being modified to default to cached mode so this env var is no longer used |
Co-authored-by: pavera <660677+pavera@users.noreply.github.com>
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.
Pull request overview
This PR removes the proxy-cached feature flag that has been rolled out and is now always enabled, simplifying the codebase by removing conditional logic around proxy caching.
Changes:
- Removed the
cachedModeparameter from theProxyBuilderconstructor - Removed the
PROXY_CACHEenvironment variable from proxy container configuration - Removed the
experiments['proxy-cached']check from theUpdater.runUpdater()method
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/updater.ts | Removed the cachedMode variable and conditional logic checking for the proxy-cached experiment flag, simplified ProxyBuilder instantiation |
| src/proxy.ts | Removed cachedMode parameter from constructor and removed PROXY_CACHE environment variable from container configuration |
| dist/main/index.js | Updated compiled JavaScript to reflect source code changes |
| tests/updater-builder-integration.test.ts | Simplified test to remove cachedMode parameter, but introduced a bug with incorrect argument order |
| tests/proxy-integration.test.ts | Simplified test to remove cachedMode parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The
proxy-cachedexperiment flag has been rolled out and is now always enabled. This PR removes the conditional logic as the proxy now defaults to cached mode.Changes:
experiments['proxy-cached']check fromUpdater.runUpdater()cachedModeparameter fromProxyBuilderconstructorPROXY_CACHEenvironment variable (no longer needed as proxy defaults to cached mode)Before:
After:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.