Rm -rf husky

[brunnre8]

@itsjohncs @MaxLeiter, can we please get rid of husky?
I don't like it that installing dependencies muck about with local git hooks.

Would you be ok with adding an explicit "hooks:install" script in package.json?
That way people who do want to have it can install it that way and I don't get emojis in my editor... it really breaks my workflow as it messes with fugitive within vim.

What's your opinion here?

[MaxLeiter]

On one hand, I don't like it and would love to remove it. On the other, I think it lowers the barrier of entry for new contributors who may be turned off by having to go back and lint themselves so CI passes. I don't care much either way, but I think if we remove it from the hooks we should remove it entirely, as the target audience won't run the hooks:install script.

[9p4]

When I was working on my PR, Husky didn't fix all of the linting issues (I still had to use yarn lint:eslint to make the CI pass).

[itsjohncs]

I haven't had any issues with The Lounge's hooks so I'm not sure what problems Husky is causing. Though I think I have had the same issue where they aren't quite extensive enough to catch all problems... not sure what that's about.

I use git hooks pretty extensively in my projects but I've never used a tool like Husky to manage them. If Husky is the problem and not the hooks, we could switch to just using bash scripts.

[brunnre8]

My problem is that running yarn install installs git hooks that are run pre-commit.
I absolutely detest that. I use git to reorganize and checkpoint my stuff and rebase etc.
I know that some things are failing and I don't care, I'll fix it up pre-push.

I also have a specific workflow with my vim integration with git... the linter thingy spews emojis into the :messages window for absolutely no gain during development.\

Having the option to install the hooks is perfectly fine, what I really want to remove is the auto install just because you fetched the deps.

Husky is the problem in the sense that it's the thing that auto installs the hooks.

[brunnre8]

@9p4 btw, lint:eslint doesn't fix your code. It's running as a linter.
In order for it to fix things it thinks are trivial, you'd need to run eslint --fix and prettier -w

[9p4]

Yes I know, I just use the linter output to check if the code will be passed in CI. Husky doesn't fix everything.

[brunnre8]

Ah I think I misunderstood what you were saying, apologies.

[itsjohncs]

I'm pretty loosely of the mind that our git hooks are probably good for getting going contributing to thelounge and I think Max was saying that as well. But having a documented route to easily disable them makes total sense to me as well. How about this as a proposition:

It looks like Husky has a built-in way to disable itself with the HUSKY env variable. Since it sounds like you never want them you could add that variable to your bash profile. We'd add a note in the readme in the dev setup section for other folks who'd like that as well.

Another option would be doing a little bit of mucking-about and make it so our hooks first check for the existence of a .disablehooks file in the repo's root (we'd add it to .gitignore as well). And again we'd add a note in the dev setup section.

How do either of these options seem?

[brunnre8]

It looks like Husky has a built-in way to disable itself with the HUSKY env variable

Strong disagreement from my side.

Installing dependencies should do nothing but install dependencies. The install hooks yarn provides are meant to compile things or build additional files, not to install git hooks.

yarn install must not install hooks, that's just broken in my personal opinion.
The readme should contain a thing that allows you to install a hook.

[itsjohncs]

What if just cloning brought in the hooks? That's the way I'm used to doing things.

[itsjohncs]

I have a git-hooks directory in the the root of many of my repos that the repo is set up to run hooks from.

[brunnre8]

That's not possible (I hope?) as that would be a huge security issue.
There are hooks that are pre-fetch or pre-clone and that would mean cloning a repo would immediately lead to arbitrary code execution.

By default the hooks directory is $GIT_DIR/hooks, but that can be changed via the core.hooksPath configuration variable (see git-config(1)).

It's the exact same problem too, leave the tooling to the person developing the code, make it easy to do the "right" thing but don't automagically do things.

[brunnre8]

that was the wrong button

[brunnre8]

As said, I'm perfectly happy with providing an explicit install command and adding the hooks into the repo, I just don't want them to get enabled by default.

[9p4]

I have just disabled git hooks entirely: git config --global core.hooksPath /dev/null

[itsjohncs]

Ha, you're totally right. I thought git config options like where to load hooks were pushed/pulled.

[brunnre8]

@9p4 the problem there is that husky will do repo local settings and happily overwrite your user / system config if we set it up the same way as now.

As said, principle of least surprise probably means that hooks should not be auto enabled.

[itsjohncs]

Hm, seems telling that you've disabled hooks for yourself @9p4, since we were hoping that the hooks were especially useful for new contributors.

🤷‍♀️ I'm fine with having them opt-in, not really something I think matters too much. I will say that I'd be sad if they were totally unavailable, since I'm personally a big fan of git-hooks and would enable them for myself. I always forget to do things and I'm happy to rely on git to make sure lint and tests get run. Even if I have to wait a little bit when committing/pushing or remember to use -n when I want to move quickly.

[9p4]

For me, it's more of a security concern. I don't want git to run anything unless if I tell it to. It is not obvious that a repo has hooks when it is first cloned (and on-clone hooks are also scary, since they can run arbitrary code before you have a chance to see what hooks are used).

[brunnre8]

I am aware that there are plenty of ways to work around the problem, but it really shouldn't be necessary.
-n only works if you don't have any hooks setup that you do want to have run and who tells you that people don't have some global hooks?

[brunnre8]

Mkey, so here's me trying to summarize the thread so that we can progress:

• Max: No strong opinion, kinda likes it for "newbies", doesn't think that those would run the install command
• John: Wants to use the hooks, would be fine with them being opt-in
• Me: (╯°□°)╯︵ ┻━┻ the auto install hooks as fast as we can

Hence, can we find a middle ground and progress with the following:

• Remove auto installation of hooks
• Add hook:install package.json script
• document said hook installation in the Readme

Would this be an acceptable path for everyone?

[MaxLeiter]

I'd add lets include a mention of the hook:install script in the README or CONTRIBUTING but beside that 👍

[brunnre8]

Ah, but of course. Updated the list