Skip to content

Conversation

@quintrino
Copy link

Currently you can't install Homebrew taps when you've set overcommit as your default template as mentioned
here (#591) and here (Homebrew/brew#4577)

This PR detects when the current git repo is being used by Homebrew and disables overcommit.

@quintrino
Copy link
Author

@sds I might need some help on figuring out how to test this, as I can't seem to wrap my head around how it's working with the existing tests and OVERCOMMIT_DISABLE.

Copy link
Owner

@sds sds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @quintrino. Can I ask why we aren't making a change in the Homebrew project to unset the GIT_TEMPLATE_DIR environment variable? It seems like this would be a problem for anyone setting a template directory, not just Overcommit users.

Appreciate any clarity you can provide. Thank you.

Overcommit::Utils.with_environment('OVERCOMMIT_DISABLE' => overcommit_disable) do
Overcommit::Utils.with_environment(
'OVERCOMMIT_DISABLE' => overcommit_disable, '
HOMEBREW_SYSTEM' => homebrew_system
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason the HOMEBREW_SYSTEM environment variable has a newline in the key name? (see the single quote above)

This likely explains why the tests don't work.

@sds sds closed this May 13, 2021
@quintrino
Copy link
Author

As an update I'm trying to get this resolved at the Brew level before fixing it at the Overcommit level.

@quintrino
Copy link
Author

Homebrew/brew#11364 is now merged into homebrew so this should resolve

Homebrew/brew#4577
#591
(both of which were raised by @boardfish and helped me explain the issue to the Homebrew core team.

@boardfish
Copy link

I'm afraid I've fallen a little out of touch with Overcommit since I originally raised that issue, but it's good to see this potentially getting fixed. Might need to jump back in with it soon. Thanks, @quintrino!

@quintrino
Copy link
Author

@broadfish Well I raised it in such a way that it addresses not just overcommit but all commit hooks so whether you're using overcommit or something else, either way you're covered. :)

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