You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
An exact copy of the PR in #7725 by @jonathandunne, but rebased against latest. Fixes#7688 Original PR description below:
Summary
apply --app-name to VS Code web bootstrap product configuration
set nameShort/nameLong so ${appName} in tab titles uses the configured app name
add a focused route test covering the injected configuration
Testing
attempted npm install (partially completed but failed in lib/vscode on missing gssapi/gssapi.h for kerberos under Node 24)
attempted npm run test:unit -- --runInBand --runTestsByPath test/unit/node/routes/vscode.test.ts test/unit/node/routes/login.test.ts
test run was blocked by a pre-existing TypeScript error in src/node/http.ts (auth.cookie-max-age missing on DefaultedArgs) before these route tests executed
I feel like it would make a lot of sense to add this to the integration.diff patch instead of a separate one. What do you think?
At a glance it appears that each file has a single block of prose text at the top describing its purpose. If we were to integrate this into integration.diff then we would have to drop the descriptive comment currently in the app-name.diff header, or figure out some other way to to include it. I feel like the human readable description being near the change is valuable.
Is there some other way to add comments to diffs? I'm not familiar with the format. We could add the description of this to the existing integration.diff header, but I feel like that would be confusing. My instinct (knowing very little about this project's structure) is that each cohesive change should be isolated into its own file, similar to how one would have a commit or PR for each change. This way when something breaks due to an upstream change, the diffs associated with it are consolidated together.
Note: I spent more time writing these two paragraphs than I did investigating, so I may be way off base or there may be an obvious solution!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An exact copy of the PR in #7725 by @jonathandunne, but rebased against latest. Fixes #7688 Original PR description below:
Summary
--app-nameto VS Code web bootstrap product configurationnameShort/nameLongso${appName}in tab titles uses the configured app nameTesting
npm install(partially completed but failed inlib/vscodeon missinggssapi/gssapi.hforkerberosunder Node 24)npm run test:unit -- --runInBand --runTestsByPath test/unit/node/routes/vscode.test.ts test/unit/node/routes/login.test.tssrc/node/http.ts(auth.cookie-max-agemissing onDefaultedArgs) before these route tests executed