-
Notifications
You must be signed in to change notification settings - Fork 6k
Only run clang-tidy in presubmit when C, C++, ObjC, or ObjC++ files change #36887
Conversation
|
Am I missing any extensions? |
| - tools/** | ||
| - ci/** | ||
| - "**.c" | ||
| - "**.cc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this also needs to run on changes to header files right? Here and below.
| - "**.cc" | |
| - "**.cc" | |
| - "**.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. Should I filter out third_party from these checks? (I mean in the script, not in the ci.yaml)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, you explicitly added a test for third_party so I guess not #26722
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the script already skips source files in third_party.
Morning brain is also remembering that we generate and analyze source for flat buffers and shader bindings, so this should also run on changes to *.fbs, *.frag, and *.vert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for my shader ignorance, so changes to those files generate .cc files that are being linted?
The list of files it's linting:
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8799745439669169009/+/u/test:_lint_ios_debug_sim/stdout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the generated source might be in header files, which is why they might not show up in the list of .cc files being analyzed. It's why we have to do builds before linting: https://flutter.googlesource.com/recipes/+/refs/heads/main/recipes/engine/engine_lint.py#91.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. That's a lot of files, but at least it will be skipped on dart and java -only commits 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
46a72a6 to
37a67f6
Compare
Only run clang-tidy builders in presubmit when C, C++, ObjC, or ObjC++ files change. Also run if the
DEPSor CI files change. Run onLinux Android clang-tidyif any python files change, since that's the shortest-running builder. All builds will always run in post-submit.To test the globs, the first commit 586b9e1 added a comment to a
.mmfile, andMac Host clang-tidydid not include.ci.yaml. It was correctly scheduled just because of the extension:https://ci.chromium.org/p/flutter/builders/try/Mac%20Host%20clang-tidy/760
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.