Codecov Report

Merging #4972 (72d97a9) into main (d22f312) will not change coverage.
The diff coverage is n/a.

❗ Current head 72d97a9 differs from pull request most recent head c2c7845. Consider uploading reports for the commit c2c7845 to get more accurate results

Impacted file tree graph

@@ Coverage Diff @@
## main #4972 +/- ##
=======================================
 Coverage 71.58% 71.58% 
=======================================
 Files 29 29 
 Lines 1675 1675 
 Branches 373 373 
=======================================
 Hits 1199 1199 
 Misses 405 405 
 Partials 71 71 

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 d22f312...c2c7845. Read the comment docs.

✨ code-server docs for PR #4972 is ready! It will be updated on every commit.

• Host: https://coder.com/docs/code-server/72d97a9
• Last deploy status: success
• Commit: c2c7845
• Workflow status: https://github.com/coder/code-server/actions/runs/1989763764

In theory, this should work 🤔 But when I search

It does publish but it's using the old name 🤔
image

I think I know why. This line:

Needs to be after the

Because the package.json has to be in the current path from where you run npm pkg. I think the best approach is to set the default name to code-server then always set it with npm pkg (but open to other ideas). Something like:

DEV_PACKAGE_NAME="code-server"
if [[ "$NPM_ENVIRONMENT" == "development" ]]; then
 DEV_PACKAGE_NAME="@coder/code-server-pr"
fi
pushd release
npm pkg set name="$DEV_PACKAGE_NAME"
npm version "$NPM_VERSION"
popd

Though if we do that, then we'll need the v7/v8 npm CLI in any place we use this script 🤔 Which is fine but I'll need to update it in the npm-brew.yaml file.

The Brew workflow will not trigger this else branch right? Since NPM_ENVIRONMENT will be PRODUCTION.

Although the npm set is a great QOL improvement, maybe we should just use jq since we already use it elsewhere hahaha

Although the npm set is a great QOL improvement, maybe we should just use jq since we already use it elsewhere hahaha

I'm all about those QOL improvements though 😛 Plus less tools/maintenance overhead only having to worry about npm

The Brew workflow will not trigger this else branch right? Since NPM_ENVIRONMENT will be PRODUCTION.

Exactly!

error Couldn't publish package: "https://registry.yarnpkg.com/@coder%2fcode-server-pr: You must sign up for private packages"
info Visit https://yarnpkg.com/en/docs/cli/publish for documentation about this command.

Looks like I may need to use a different npm token 🤔

Plus less tools/maintenance overhead only having to worry about npm

I think this is the main (only?) argument in favor, but since we already have jq as a dep it weakens the argument in this particular case (it also makes devs have to worry about updating npm which ends up adding burden for existing devs).

I think this is the main (only?) argument in favor, but since we already have jq as a dep it weakens the argument in this particular case

Fair point! If I use jq, I don't have to use npm v7 or above, so that'd save time in the install steps. And we're all in favor of faster workflows

I think either way is not a big deal, the only pain point I can think of is if I have to release manually (like what happened last release 😢) then I would need to update node/npm so I can run it then downgrade it again (since code-server needs a lower version).

I think either way is not a big deal, the only pain point I can think of is if I have to release manually (like what happened last release 😢) then I would need to update node/npm so I can run it then downgrade it again (since code-server needs a lower version).

Well technically you could upgrade npm only with npm i -g npm@7. Using a newer version of npm wouldn't affect your use of code-server, but agreed, it is one extra step and we should avoid it if we can!

Well technically you could upgrade npm only with npm i -g npm@7. Using a newer version of npm wouldn't affect your use of code-server, but agreed, it is one extra step and we should avoid it if we can!

🤯 Good point! That sounds like a pretty reasonable workaround IMO

🤯 Good point! That sounds like a pretty reasonable workaround IMO

Well...you convinced me to use jq so I'm changing it 🤣 In all honesty though, you're right in that we should limit dependencies and using jq would be a smaller code change, which I think we're all in favor of. No biggie.

hahahahahaha well here is a heart for you <3

I look forward to the day we can remove jq tbh, maybe by writing the scripts in TypeScript or calling out to Node or something

@code-asher bump