[clang] [llvm] [clang-format] Add CI check confirming ClangFormatStyleOptions.rst is up-to-date. (PR #111513)

Owen Pan via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 11 23:10:59 PDT 2024


owenca wrote:

> > IMO it would be more helpful to add the build target to lib/Format/CMakeLists.txt instead. For example,
> 
> ```
> $ ninja FormatTests
> ```
> 
> Moving the target from `clang/docs/CMakeLists.txt` to `lib/Format/CMakeLists.txt` sounds great to me, but the testing part of the thing relies on us being in a `git clone`, it's a very CI-oriented thing, my impression is that it's more appropriate for it to live in the workflow yaml files. I'm saying this because you suggested `FormatTests` as the name... I'm not sure if `clang-format-style-options` is a good name, but I think it's closer to what we can do there. What do you think?

I wasn't suggesting a different name as `FormatTests` is an existing build target. See https://github.com/llvm/llvm-project/blob/0fba8381d2a71ff440fdf0ae30d59a0bf07fea75/clang/unittests/Format/CMakeLists.txt#L41

Actually, I like the `clang-format-style-options` name.

Anyway, my current workflow is:
1. Edit files in `clang/lib/Format` and `clang/unittests/Format`.
2. Edit `clang/include/clang/Format/Format.h` if needed.
3. Run `ninja FormatTests`.
4. Run `cd clang/docs/tools && dump_format_style.py` if Step 2 above wasn't skipped.
5. Run the unit tests.
6. Run `git commit -a`.

It would be nice if I didn't have to run step 4 manually.

> > I'm ok with not adding the CI check though.
> 
> IMO that would be sad, I would really like to free maintainers up from having to remember this dependency themselves. Is this because you feel `.github/workflows/docs.yml` is the wrong place for that validation? I see that step more as a pre-build validation for docs than as a test, that's why `docs.yml` looks like a good home for it from my point of view, but we could move it elsewhere if you're uncomfortable with that.

I'm not opposed to adding the CI check either.

https://github.com/llvm/llvm-project/pull/111513


More information about the cfe-commits mailing list