Skip to content

fix: minor build fixes - #5039

Merged
code-asher merged 3 commits into
coder:mainfrom
code-asher:build-fixes
Mar 29, 2022
Merged

code-asher merged 3 commits into
coder:mainfrom
code-asher:build-fixes

Conversation

@code-asher

@code-asher code-asher commented Mar 29, 2022 •

Copy link
Copy Markdown
Member

See commits for list of fixes.

The lines were a bit off.
This gets overidden in the standalone but it was getting uselessly
included in the npm package.
@code-asher
code-asher requested a review from a team March 29, 2022 17:40

@jsjoeio jsjoeio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might be worth adding a small description to the PR as well

Comment thread ci/build/build-release.sh
Comment on lines +81 to +85
# - Some extensions have a .gitignore which excludes their built source from
# the npm package so exclude any .gitignore files.
# - Exclude Node as we will add it ourselves for the standalone and will not
# need it for the npm package.
rsync -avh --exclude .gitignore --exclude /node ./lib/vscode-reh-web-*/ "$VSCODE_OUT_PATH"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this fix #5019?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am not seeing a .gitignore in the typescript-language-features extension so probably not unfortunately 😢

Will comment more on that issue; planning on getting through all my GitHub notifs today

Comment thread patches/last-opened.diff
--- code-server.orig/lib/vscode/src/vs/code/browser/workbench/workbench.ts
+++ code-server/lib/vscode/src/vs/code/browser/workbench/workbench.ts
@@ -350,19 +350,6 @@ class WorkspaceProvider implements IWork
@@ -410,19 +410,6 @@ class WorkspaceProvider implements IWork

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this fix that weird space thing we were seeing?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am not sure if we are talking about the same thing but this fixes the warning output by quilt push -a since the patch is slightly off.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes, exactly!

@codecov

codecov Bot commented Mar 29, 2022

Copy link
Copy Markdown

Codecov Report

Merging #5039 (d374e03) into main (326a1d1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5039   +/-   ##
=======================================
  Coverage   71.30%   71.30%           
=======================================
  Files          30       30           
  Lines        1683     1683           
  Branches      373      373           
=======================================
  Hits         1200     1200           
  Misses        413      413           
  Partials       70       70           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 326a1d1...d374e03. Read the comment docs.

@code-asher
code-asher merged commit 18e19d2 into coder:main Mar 29, 2022
@code-asher
code-asher deleted the build-fixes branch March 29, 2022 18:13
TinLe pushed a commit to TinLe/code-server that referenced this pull request Apr 23, 2022
* Regenerate last opened patch

The lines were a bit off.

* Remove packaged .gitignore files

Fixes coder#4964.

* Remove extra Node binary

This gets overidden in the standalone but it was getting uselessly
included in the npm package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants