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

Removed snapUpdate.sh and replaced with inline command #65579

Merged
merged 1 commit into from Jan 28, 2019

Conversation

Projects
None yet
3 participants
@Kedstar99
Copy link

Kedstar99 commented Dec 22, 2018

This patch removes the need for snapUpdate.sh. This has the following benefits:
+Simplifies the build process by avoiding setting and moving the file.
+Avoids creating a new shell that simply spawns another shell environment.
+Simplifies the update mechanism by removing the need to track the existence of snapUpdate.sh.
+Keeps the logic for snapUpdate.sh close to updateService.snap.ts

I have tested the code changes to spawn within a Node environment in vscode console and it correctly spawns another instance of vscode.

If this patch isn't wanted, there are still some changes that may be suggested. For example, currently updateService.linux.ts is calling snapUpdate.sh but the default Linux builds aren't being built with the script. Similarly, the comment for executing snapUpdate.sh isn't correct.

Running the command 'grep -nr --exclude-dir=".git" 'snapUpdate.sh' .' produces no results so I assume I have removed all references.

@Kedstar99 Kedstar99 force-pushed the Kedstar99:snapFix branch 2 times, most recently from 7454f38 to 296b8fb Dec 22, 2018

Krish De Souza
Removed snapUpdate.sh and replaced with inline command
+Simplifies the build process.
+Avoids creating a new shell that simply spawns another shell environment.
+Simplifies the update mechanism by removing the need to track the existence of snapUpdate.sh.
+Keeps the logic for snapUpdate.sh close to updateService.snap.ts

@Kedstar99 Kedstar99 force-pushed the Kedstar99:snapFix branch from 296b8fb to 4c02c55 Dec 22, 2018

@joaomoreno joaomoreno added this to the December/January 2019 milestone Jan 3, 2019

@joaomoreno joaomoreno merged commit eb69d48 into Microsoft:master Jan 28, 2019

2 checks passed

VS Code #20181222.7 succeeded
Details
license/cla All CLA requirements met.
Details
@joaomoreno

This comment has been minimized.

Copy link
Member

joaomoreno commented Jan 28, 2019

Yeah let's take this! Thanks! 🍻

@Kedstar99 Kedstar99 deleted the Kedstar99:snapFix branch Jan 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment