microsoft / wil

Windows Implementation Library
MIT License
2.58k stars 236 forks source link

clang-format instructions are incomplete #417

Closed asklar closed 10 months ago

asklar commented 10 months ago

hit some hurdles trying to make the CI happy (new CI check to validate formatting): PoV: (Windows, msvc, msbuild)-centric

  1. clang-format.exe is not in the PATH by default, had to:

    path %path%;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\Llvm\x64\bin
  2. git clang-format does not work image

The reason AFAICT is that this command is an extension provided by the llvm/clang install/repo via the git-clang-format.bat script: https://github.com/llvm/llvm-project/blob/main/clang/tools/clang-format/git-clang-format.bat

  1. git-clang-format relies on a python install, which raises the requirement bar

expected: golden path with standard "windows dev" tooling (msvc, msbuild, vs)

dunhor commented 10 months ago

RE (1) - Interesting, I would have assumed that the VS command window would have added that to the PATH, but I guess not. More reason to suggest using the scripts I suppose. I wonder if it might be worth adding a separate script specifically for adding the VS-shipped clang-format to the PATH. That would be generally useful since that's what we expect anyway.

RE (2) - I was curious about that, but had no way to validate since I instinctively install LLVM on any new machine I'm setting up. We should update the docs to mention the prerequisite for using git clang-format.

RE (3) - Also good to know; I don't recall coming over this being listed as a requirement in the LLVM docs, but I guess I already have python installed, and I assume that the CI machines have it pre-installed, too. That should be called out along with (2).

As an aside, I also want to play around with including the modified changes in the build output. The simplest thing to do would be to just run git diff on the CI machine, though idk how useful that info would be in that format. Another thing I want to look into assuming I get the time is to figure out how to add a command to PRs to format & submit the results. E.g. something like /azp format, if that's possible.