Closed christianbundy closed 3 years ago
I'm of two minds here...
On the one hand, yes, that's annoying.
On the other hand... I remember making my first commits and being confused and frustrated with git gui
suddenly freezing etc...
Thanks for the feedback. I'm surprised that git gui
froze, what's that about? Was it running the pre-commit hook but not giving you any info about that?
Yes exactly. It was just unexpected and git gui is pretty basic UI after all.
What do you think the best way to solve that would be? The new common-good (not merged yet) is super fast, and only tests the files you've staged, which should help -- maybe a note in the docs about the pre-commit hook? It seems like every time someone makes their first contribution we have to go over the npm run fix
thing, and it'd be awesome to avoid that if possible without screwing power users with git gui
.
Yeah I think a good ALL CAPS BOLD WARNING in Contributing.md might be enough.
What were the changes you force-pushed?
just a rebase. I find merging from master into PRs exceedingly annoying to look at in the commit log :D
In the future could we talk about that first? I was surprised that you force-pushed to my repo, especially with commits that modify my commit history.
Yes, sorry about that. I had kinda gotten used to the workflow from work, but you're right it's not the same here. Won't happen again :)
I think next time I'll either comment & wait, or just make a new PR and @ you. That seems more polite than pushing to your repo. :P
No biggie, thanks! :heart:
What's the problem you solved?
Problem: I keep forgetting to run tests and my commits aren't actually passing in CI. I'd really like for these to be run automatically so that I don't have to remember my pre-commit hooks.
What solution are you recommending?
Solution: Revert commit d9e829e2e8eddcc7c8edae2dd542cb69d86836c1, which Nick Wynja added to remove Husky as a pre-commit hook. I hope that anyone who doesn't want to run the pre-commit hook will run Git with the
--no-verify
option, and anyone bummed at this change will open a conversation to discuss in more details (or just revert this commit).See-also: https://github.com/fraction/oasis/issues/346