Codecov Report

Merging #5518 (ae4d210) into main (101d4ee) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #5518   +/-   ##
=======================================
  Coverage   72.44%   72.44%           
=======================================
  Files          30       30           
  Lines        1673     1673           
  Branches      366      366           
=======================================
  Hits         1212     1212           
  Misses        398      398           
  Partials       63       63           

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 101d4ee...ae4d210. Read the comment docs.

Actually... Where's my head. Forgot about the --unsafe-perm that npm requires... Updating coming up!

Actually... Where's my head. Forgot about the --unsafe-perm that npm requires... Updating coming up!

Nice catch! Remind me again, why do we need this?

Nice catch! Remind me again, why do we need this?

https://github.com/coder/code-server/blob/main/ci/build/npm-postinstall.sh#L95-L101

Now, is it actually needed? Not sure. Definitely something I'm happy to look into in the future.

I honestly can't remember. @code-asher might know. Probably fine to add for now then we can revert/fix if needed.

I think we only need it if someone is installing code-server with root because NPM will drop permissions in the post install scripts making the code-server install fail.

I think we only need it if someone is installing code-server with root because NPM will drop permissions in the post install scripts making the code-server install fail.

So we should add a check for the user running the script, and only show the warning if it's root? Happy to play around with that and send a separate PR for that in the next couple of days.