Some feedback:
I notice that you have some test methods in here, I typically would rather have a subclass for testing purposes to make it clearer that a method is only used from tests (that is more a personal taste).
You could use process.type to check if you are in the renderer or browser process. Though given my other feedback of splitting services up, I think the renderer should not have a dependency to this backup workspace management service.
You should not call fs.existsSync(), just call fs.readFile() and handle the error. That saves you one fs call.
The setCurrentWorkspace is very ugly. Instead I would pass in the IWorkspaceContextService as optional dependency and get the workspace from there. Though again, given my feedback on splitting up services, I think this method is a good indication that we need 2 services: one for main side to manager backup workspaces and one for the renderer to backup files.
Some feedback:
I notice that you have some test methods in here, I typically would rather have a subclass for testing purposes to make it clearer that a method is only used from tests (that is more a personal taste).
You could use
process.typeto check if you are in the renderer or browser process. Though given my other feedback of splitting services up, I think the renderer should not have a dependency to this backup workspace management service.You should not call
fs.existsSync(), just callfs.readFile()and handle the error. That saves you one fs call.The
setCurrentWorkspaceis very ugly. Instead I would pass in theIWorkspaceContextServiceas optional dependency and get the workspace from there. Though again, given my feedback on splitting up services, I think this method is a good indication that we need 2 services: one for main side to manager backup workspaces and one for the renderer to backup files.