Skip to content

Conversation

@NickGerleman
Copy link
Contributor

@NickGerleman NickGerleman commented Apr 15, 2020

I don't think this fully works correcty, and unlike lint-staged, yarn format has a pretty significant latency. Remove the hook for now.

Microsoft Reviewers: Open in CodeFlow

I don't think this fully works correcty, and unlike lint-staged, `yarn format` has a pretty significant latency. Remove the hook for now.
@NickGerleman NickGerleman requested a review from a team as a code owner April 15, 2020 12:24
Copy link
Member

@vmoroz vmoroz left a comment

Choose a reason for hiding this comment

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

:shipit:

@acoates-ms
Copy link
Contributor

Yeah I think the issue is that just doing yarn format tries to run on all the files in the repo. Really a hook like this should only run on the files you are commiting. -- Then it would be fast enough. It also seems to run async, which means the formatting changes would miss the commit, which kind of defeated the purpose.

I'd prefer a fix for the above, but removing for now is fine.

@acoates-ms
Copy link
Contributor

Can we remove husky as a dep if we are not using it?

@NickGerleman
Copy link
Contributor Author

@acoates-ms IIRC the non CI variant of yarn format does already only scan files that are staged, or at least tries to have some optimization. We've seen some odd behavior with everything happening before though, where I wonder if there is some standard tooling we could utilize instead of our scripts.

I can remove Husky. I thought to leave it bc we would add something else, but it seems easy enough to just add it back then.

@NickGerleman NickGerleman merged commit d04b28b into microsoft:master Apr 17, 2020
asklar pushed a commit to asklar/react-native-windows that referenced this pull request Apr 19, 2020
* Remove Clang Format Pre-Commit Hook

I don't think this fully works correcty, and unlike lint-staged, `yarn format` has a pretty significant latency. Remove the hook for now.

* Remove Husky
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