Skip to content
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

vsce: patch release v2.2.13 #43204

Merged
merged 2 commits into from Oct 20, 2022
Merged

vsce: patch release v2.2.13 #43204

merged 2 commits into from Oct 20, 2022

Conversation

abeatrix
Copy link
Contributor

@abeatrix abeatrix commented Oct 19, 2022

Follow up on #43060 - moving tokens from user settings to secret storage

Update 1

The vsce package output was not working as intended in the previous PR, so I've fixed that (users were not logged in automatically) and made some improvements in the general user experience about logging in and out (slient pop-ups when login in).

See Loom video demo: https://www.loom.com/share/d475b8898d4c4c6eacbd4b82fe5dc698

Update 2

The release pipeline was also broken which @philipp-spiess and I were able to dive into and got them to work locally at least. For example:

sourcegraph/client/vscode on  b/vsce [!] via  v16.13.2 on ☁️  (us-west-1) on ☁️  beatrix@sourcegraph.com
❯ yarn package
✔  success   Code-intel-extensions for revision "v3.41.1" are already bundled.
✔  success   Code-intel-extensions for revision "v3.41.1" are already bundled.
[13:00:14] Using gulpfile ~/Dev/sourcegraph/client/vscode/gulpfile.js
[13:00:14] Starting 'webpack'...
[13:02:38] extension:node:
  extension:node compiled in 95656 ms

extension:webworker:
  extension:webworker compiled in 95633 ms

webviews:
  WARNING in ../../node_modules/react-router-dom-v5-compat/index.js 422:15-25
  export 'Action' (imported as 'Action') was not found in 'history' (possible exports: __esModule, createBrowserHistory, createHashHistory, createLocation, createMemoryHistory, createPath, locationsAreEqual, parsePath)

  WARNING in ../../node_modules/react-router-dom-v5-compat/node_modules/react-router/index.js 12:0-74
  export 'Action' (reexported as 'NavigationType') was not found in 'history' (possible exports: __esModule, createBrowserHistory, createHashHistory, createLocation, createMemoryHistory, createPath, locationsAreEqual, parsePath)

  WARNING in ../../node_modules/react-router-dom-v5-compat/node_modules/react-router/index.js 822:21-31
  export 'Action' (imported as 'Action') was not found in 'history' (possible exports: __esModule, createBrowserHistory, createHashHistory, createLocation, createMemoryHistory, createPath, locationsAreEqual, parsePath)

  webviews compiled in 143886 ms
[13:02:38] Finished 'webpack' after 2.4 min
This extension consists of 293 files, out of which 227 are JavaScript files. For performance reasons, you should bundle your extension: https://aka.ms/vscode-bundle-extension . You should also exclude unnecessary files by adding them to your .vscodeignore: https://aka.ms/vscode-vscodeignore
 DONE  Packaged: dist/sourcegraph-2.2.13.vsix (293 files, 12.93MB)
 INFO
The latest version of vsce is 2.12.0 and you have 2.7.0.
Update it now: npm install -g vsce

Test plan

Tested with package output here:
sourcegraph-2.2.13.vsix.zip

Integration test is also passing:

❯ yarn test-integration
Not in BUILDKITE. No annotation will be generated in ./annotations

  VS Code extension
...
    Identifiers: {
      method: 'POST',
      body: '{"query":"\\n            query ImplementationsIntrospectionQuery {\\n             '... 182 more characters,
      url: 'https://sourcegraph.com/.api/graphql?ImplementationsIntrospectionQuery'
    }
      ✓ works (19697ms)


    1 passing (24s)

App preview:

Check out the client app preview documentation to learn more.

@abeatrix abeatrix requested review from philipp-spiess and a team October 19, 2022 21:56
@cla-bot cla-bot bot added the cla-signed label Oct 19, 2022
@sg-e2e-regression-test-bob

Bundle size report 📦

Initial size Total size Async size Modules
0.00% (0.00 kb) 0.00% (0.00 kb) 0.00% (0.00 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits 8f65c35 and b6ee0fc or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

@vdavid
Copy link
Contributor

vdavid commented Oct 19, 2022

@abeatrix could you elaborate on the improvements around logging in and out? It's quite a vague description, and there are a lot of various changes, and I'm finding it hard as the reviewer to judge which ones make sense. If there are UI changes, some screenshots or a Loom video would help, too.

@abeatrix
Copy link
Contributor Author

@abeatrix could you elaborate on the improvements around logging in and out? It's quite a vague description, and there are a lot of various changes, and I'm finding it hard as the reviewer to judge which ones make sense. If there are UI changes, some screenshots or a Loom video would help, too.

Oh yea i'm recording a loom video right now sorry about that, wasn't expected anyone to look at this PR til tomorrow 🤣 Will post in 10 mins!

@abeatrix
Copy link
Contributor Author

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const packageJson: any = JSON.parse(originalPackageJson)
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
packageJson.name = 'sourcegraph'
fs.writeFileSync('package.json', JSON.stringify(packageJson))

childProcess.execSync('yarn vsce package --yarn --allow-star-activation -o dist', { stdio: 'inherit' })
childProcess.execSync('vsce package --yarn --allow-star-activation -o dist', { stdio: 'inherit' })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure if this would work on CI (vsce might not be in the PATH). Does it work to run yarn vsce here considering you moved vsce to be a dependency of this project? I guess we can also just try this on CI and if it works we're good!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I run it locally it was giving me the error message about duplicated workspace name --do you get the same error as well? It might not work the same on CI though so we can definitely make the change again if it doesn't run, what do you think?

@abeatrix abeatrix merged commit ea4d1f8 into main Oct 20, 2022
@abeatrix abeatrix deleted the b/vsce branch October 20, 2022 15:54
@abeatrix
Copy link
Contributor Author

For future references, vsce gives out a warning when running the package command: The latest version of vsce is 2.12.0 and you have 2.7.0.

However, updating the version to 2.12.0 resulted in the following error:

❯ vsce package --yarn --allow-star-activation
 ERROR  Command failed: yarn list --prod --json

I believe this is due to the limitation of vsce not supporting yarn v2: microsoft/vscode-vsce#517

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants