Edward Chen

FWIW, I think this is still an unanswered question. The comments haven't been clarified yet: https://github.com/onnx/onnx/blob/a252e651fda8e73935827505a148146459c2e06d/onnx/onnx.in.proto#L517-L524

> > @snnn With this test PR I am seeing a few checks that are particularly noisy / may not make sense (`onnxruntime::Tensor* tensor = nullptr = value.GetMutable();`) > >...

> > Is the compile_commands.json file getting generated and passed to clang-tidy (-p option)? > > No since I wasn't sure what the best way is to generate it. Is...

FYI, these warnings seem incorrect: https://github.com/microsoft/onnxruntime/pull/12655#discussion_r979134421 https://github.com/microsoft/onnxruntime/pull/12655#discussion_r979134422 https://github.com/microsoft/onnxruntime/pull/12655#discussion_r979134423 https://github.com/microsoft/onnxruntime/pull/12655#discussion_r979134424 https://github.com/microsoft/onnxruntime/pull/12655#discussion_r979134426

> @edgchen1 that's a little more noise than what I hope it would produce! Should I turn off > > ``` > misc-unused-parameters > readability-convert-member-functions-to-static > cppcoreguidelines-init-variables > ``` >...

https://github.com/microsoft/onnxruntime/actions/runs/3092428102/jobs/5003682677 looking at the build output, it appears that there are no files checked. perhaps the filter should be adjusted? ``` Run ZedThree/[email protected] ... Diff from GitHub PR: [, ]...

> @edgchen1 I think that's because the lint is only checking diffs! ah ok, that makes sense. as a side note, we'll probably need to generate a compile_commands.json file to...

> > as a side note, we'll probably need to generate a compile_commands.json file to configure clang-tidy properly. > > Absolutely. I haven't figured out how to do that though....

> Strange: https://github.com/microsoft/onnxruntime/actions/runs/3099926945/jobs/5019640574#step:8:39 > > clang-tidy failed with return code 134 and error: LLVM ERROR: Cannot chdir into "/home/runner/work/onnxruntime/onnxruntime/build/Debug"! wonder if setting base_dir is needed since compile_commands.json is generated outside...

> @edgchen1 @cloudhan should > > modernize-use-nodiscard be disabled? Otherwise this is ready to merge. > >  I can see the merits of using [[nodiscard]] more. However, we don't...