Configure Watchman during ConfigStep #299
Conversation
kewillford
left a comment
kewillford
left a comment
There was a problem hiding this comment.
Looks good.
As far as the hooks question. Yes we should do something there. It should continue to work with v1 of the hooks but it would be nice if we would update to v2. With VFSForGit we would copy the hooks from the install location at mount time so that we had the up to date hooks. We could resurrect that code and run it here in the config step I guess but I would really like to use a native hook that talks directly to the watchman server instead of the perl script that launches watchman commands.
| return true; | ||
| } | ||
|
|
||
| protected override void PerformMaintenance() |
There was a problem hiding this comment.
We could plumb the TextWriter through so that we could have it here or set it when the Step is constructed so that we could write to the output of the console if it wasn't null but that seems like overkill for a message most users won't care about.
There was a problem hiding this comment.
I thought the same. If we wanted that, then we would pass it through in the constructor.
Signed-off-by: Derrick Stolee <[email protected]>
2 participants
Resolves #291.
Mostly a code move from
CloneVerbtoConfigStep. We lose some output during clone, like "Watchman configured!" as we don't have access toOutputduring theConfigStep. TheCloneVerbruns theConfigStepdirectly, but so does theRegisterStepand background maintenance.There is a benefit, though: we will try configuring Watchman on every upgrade and once a day. If a user installs Watchman after cloning, then we will configure it automatically.
One remaining thing to think about: we are currently copying the existing hook template from the hooks directory. As this changes (see microsoft/git#225 and gitgitgadget/git#510), we will need a different way to populate the hook. Perhaps we should store the hook contents in the Scalar codebase to avoid using a stale (or malicious) hook. Cc @kewillford for thoughts on this.