microsoft / scalar

Scalar: A set of tools and extensions for Git to allow very large monorepos to run on Git without a virtualization layer
MIT License
1.39k stars 63 forks source link

Delete redundant service verb #406

Closed mjcheetham closed 4 years ago

mjcheetham commented 4 years ago

Delete the ServiceVerb which is now redundant; the ListVerb performs the exact same actions.

mjcheetham commented 4 years ago

A quick git grep service found an interesting case hat might cause some problems:

Scalar.Installer.Windows/Setup.iss:  Exec('scalar.exe', 'service --help', '', SW_HIDE, ewWaitUntilTerminated, ResultCode);

Likely this is a simple replacement, like s/service --help/list --help/.

The "TODO" above this is interesting:

procedure StopMaintenanceTasks();
var
  ResultCode: integer;
begin
  // TODO: #185 Instead of calling --help, use the correct action for stopping the
  // maintenance task
  Exec('scalar.exe', 'service --help', '', SW_HIDE, ewWaitUntilTerminated, ResultCode);
end;

That issue (#185) was closed by @derrickstolee saying:

Should not be needed with #236. Closing.

..but looking at #236, it appears this was closed without a code change. You said:

If we run the maintenance tasks in process, then we cannot authenticate with the remote. This makes the fetch-commits-and-trees step impossible and the commit-graph step likely to fail.

Do we still need to implement this TODO and stop maintenance tasks? Don't we really just need a new verb scalar stop [--force/confirm etc] that will stop existing tasks?

I can replace this with a dummy command again for now, but feels like perhaps #185 should be reopened, no?

derrickstolee commented 4 years ago

I can replace this with a dummy command again for now, but feels like perhaps #185 should be reopened, no?

I agree with your investigation and conclusion. Thanks!

mjcheetham commented 4 years ago

I can replace this with a dummy command again for now, but feels like perhaps #185 should be reopened, no?

I agree with your investigation and conclusion. Thanks!

Reopened the issue: https://github.com/microsoft/scalar/issues/185#issuecomment-658854667