Skip to content

scripts/build: ditch -vsc suffix #1310

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

Closed
wants to merge 4 commits into from
Closed

Conversation

sr229
Copy link
Contributor

@sr229 sr229 commented Jan 28, 2020

This would fix some automation systems since keeping the vsc suffix with our new tag deployment system breaks some of the CIs most people use.

This should reproduce quite predictable builds like we did previously.

image

This compliments GH-1303.

This would fix some automation systems since keeping the vsc suffix with our new tag deployment system breaks some of the CIs most people use.
@sr229 sr229 self-assigned this Jan 28, 2020
@sr229 sr229 added the enhancement Some improvement that isn't a feature label Jan 28, 2020
This is to add some sort of seperation between the version and the name of the binary.
Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

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

Let's update scripts/test.sh to just check that the version output starts with "info". That's probably enough to ensure the binary packaged correctly and is serving code-server. Seems I forgot to add the test step to Travis which is why it passed.

scripts/build.ts Outdated
@@ -231,7 +231,7 @@ class Builder {

const [productJson, packageJson] = await Promise.all([
merge("product", { commit, date }),
merge("package", { codeServerVersion: `${codeServerVersion}-vsc${vscodeVersion}` }),
merge("package", { codeServerVersion: codeServerVersion }),
Copy link
Member

Choose a reason for hiding this comment

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

Not a big deal but this could be { codeServerVersion }.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch

@code-asher
Copy link
Member

Also I think we should add a line to --version that shows the VS Code version.

@sr229
Copy link
Contributor Author

sr229 commented Jan 29, 2020

Also I think we should add a line to --version that shows the VS Code version.

Point me to the right direction @code-asher, I don't think I know where.

Ayane Satomi added 2 commits January 29, 2020 22:35
also added a sanity check for daily version print

Signed-off-by: Ayane Satomi <[email protected]>
@sr229
Copy link
Contributor Author

sr229 commented Jan 29, 2020

Just a little note I can't push to the repository at the moment, but I have the changes ready to merge.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 5, 2020

@code-asher says he already merged into #1338

@nhooyr nhooyr closed this Feb 5, 2020
@nhooyr nhooyr deleted the sr229/ditch-vsc-suffix branch February 5, 2020 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Some improvement that isn't a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants