Skip to content
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

[WIP] - [EXPERIMENTAL] Multiple instance support #847

Closed
wants to merge 6 commits into from
Closed

[WIP] - [EXPERIMENTAL] Multiple instance support #847

wants to merge 6 commits into from

Conversation

auxves
Copy link
Contributor

@auxves auxves commented Apr 17, 2019

Short description of what this resolves:

This PR fixes issues with multiple instances running at the same time.

Changes proposed in this pull request:

  • Only allow one instance of VS Code to auto-upload
  • Catch errors with lock file

Fixes: #839

How Has This Been Tested?

I have tested it by installing it and running multiple instances of VS Code. However, I could have missed something, so it needs a lot more testing before even thinking about merging it.

Checklist:

  • I have read the contribution guidelines.
  • Auto upload only from the first instance of VS Code
  • Somehow update configuration when the auto upload instance is closed
  • Lots of testing

commons.ts
----
Instantiate autoUploadService with context
Return null instead of setting variable to null and returning it

lockfile.ts
----
Check if file exists to prevent unnecessary writes and potential issues

settings.ts
----
Ignore all state files

sync.ts
----
Catch errors with lock file
Remove unnecessary code that might cause issues

autoUploadService.ts
----
Add unique id for original instance
Only allow original instance to auto upload
Catch errors with lock file
Wait for update settings to finish before unlocking

package.json
----
Add @types/lockfile for better autocompletion
@shanalikhan shanalikhan changed the base branch from v3.2.9 to v3.3.0 April 19, 2019 10:06
@kinghat
Copy link

kinghat commented Apr 25, 2019

would it be possible to signal the other instances that there has been an update and to trigger a download on the other instances?

@auxves
Copy link
Contributor Author

auxves commented Apr 25, 2019

@k1nghat This PR only has to do with auto upload. Downloading is not affected by any of the changes proposed here.

@shanalikhan shanalikhan changed the title [EXPERIMENTAL] Multiple instance support [WIP] - [EXPERIMENTAL] Multiple instance support May 17, 2019
@shanalikhan
Copy link
Owner

@arnohovhannisyan do you think we can improve further on this PR ? Or should i go ahead and merge it.

@auxves
Copy link
Contributor Author

auxves commented May 21, 2019

@shanalikhan it definitely needs more work.

@shanalikhan
Copy link
Owner

it definitely needs more work.

What do you suggest, can we include this in release of v3.3.0 if you can complete the rest of the work by testing.

@auxves
Copy link
Contributor Author

auxves commented Jun 11, 2019

@shanalikhan If there are no issues after testing, then yes.

@auxves
Copy link
Contributor Author

auxves commented Jun 14, 2019

@shanalikhan I'm going to open a new PR with these changes because this one currently has old code from v3.2.9.

@auxves auxves closed this Jun 14, 2019
@auxves auxves deleted the fix-839 branch June 19, 2019 11:05
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.

3 participants