Skip to content

Bundle Codicons using Webpack#1482

Merged
koesie10 merged 4 commits intomainfrom
koesie10/bundle-codicons
Sep 1, 2022
Merged

Bundle Codicons using Webpack#1482
koesie10 merged 4 commits intomainfrom
koesie10/bundle-codicons

Conversation

@koesie10
Copy link
Copy Markdown
Member

This will include the Codicons inside the webview bundle, reducing the number of files that need to be loaded and the resource roots that need to be included.

The only place where a codicon is currently used is in the repository search bar for MRVA results.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

This will include the Codicons inside the webview bundle, reducing the
number of files that need to be loaded and the resource roots that need
to be included.
Base automatically changed from koesie10/unified-webpack-bundle to main August 30, 2022 09:29
@koesie10 koesie10 requested a review from a team August 31, 2022 09:49
@koesie10 koesie10 marked this pull request as ready for review August 31, 2022 09:49
@koesie10 koesie10 requested a review from a team as a code owner August 31, 2022 09:49
Copy link
Copy Markdown
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

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

With this change, does this mean that we can remove the references to codicons when generating the webview? LIke here

Uri.file(path.join(ctx.extensionPath, 'node_modules/@vscode/codicons/dist')),
and here
ctx.asAbsolutePath('node_modules/@vscode/codicons/dist/codicon.css')
?

@koesie10
Copy link
Copy Markdown
Member Author

koesie10 commented Sep 1, 2022

Thanks for noticing, those changes probably got lost when I merged in main. Those can indeed be removed.

Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

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

Thanks for tidying this up! Just a couple of questions/comments.

Comment thread extensions/ql-vscode/gulpfile.ts/webpack.config.ts
Comment thread extensions/ql-vscode/gulpfile.ts/webpack.config.ts
@koesie10 koesie10 merged commit 0c89df9 into main Sep 1, 2022
@koesie10 koesie10 deleted the koesie10/bundle-codicons branch September 1, 2022 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants