Skip to content

Conversation

@SamBent
Copy link
Contributor

@SamBent SamBent commented Dec 14, 2021

Fixes Issue

Main PR

Description

Improvements to the hook that deters users from changing generated files:

  1. The hook calls a file-comparison tool that expects its inputs to be sorted, but doesn't sort the inputs ("comm: file 1 is not in sorted order"). Fixed that by (a) sorting the dynamic list of staged files, and (b) pre-sorting the fixed list of generated files.
  2. Revised the error output to give a better suggestion about what to do next.

To get the full benefit of this change, copy eng\WpfArcadeSdk\tools\pre-commit.githook to .git\hooks\pre-commit, overwriting the existing file (both paths are relative to your repo's root). There's probably some way to cause this to happen automatically, but I couldn't find it.

Customer Impact

No change to product code. Developer experience is improved.

Regression

No.

Testing

It works for me.

Risk

No risk.

@SamBent SamBent requested a review from a team as a code owner December 14, 2021 01:45
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Dec 14, 2021
@ghost ghost requested review from fabiant3 and ryalanms December 14, 2021 01:45
@dipeshmsft
Copy link
Member

The eng\WpfArcadeSdk\tools\pre-commit.githook file is compied to .git\hooks\pre-commit at the time when build.cmd is run. It executes restore-toolset.ps1 which copies the pre-commit hook file.

@dipeshmsft dipeshmsft self-requested a review December 14, 2021 11:52
@SamBent
Copy link
Contributor Author

SamBent commented Dec 14, 2021

@dipeshmsft

The eng\WpfArcadeSdk\tools\pre-commit.githook file is compied to .git\hooks\pre-commit at the time when build.cmd is run. It executes restore-toolset.ps1 which copies the pre-commit hook file.

No it doesn't. This only happens the very first time you build the repo. After that, restore-toolset says:

Detecting WPF Git hooks...
Detected existing WPF Git pre-commit hook.

However, deleting .git\hooks\pre-commit will install the new one the next time you build:

Detecting WPF Git hooks...
Installing WPF Git pre-commit hook...

BTW, I pushed the wrong version of pre-commit.githook, lacking the sort (step 1a). Fixed in latest commit.

@SamBent SamBent merged commit 4c2cd1a into dotnet:main Dec 15, 2021
@SamBent SamBent deleted the PrecommitGithook branch December 15, 2021 00:58
@ghost ghost locked as resolved and limited conversation to collaborators Apr 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants