-
Notifications
You must be signed in to change notification settings - Fork 225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ci update #898
Ci update #898
Conversation
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.
If this PR isn't going to include build, can you remove the "Closes" part so it doesn't close that issue until it's actually fixed?
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @aquan9)
.github/workflows/clang-format-check.yml
line 11 at r1 (raw file):
matrix: path: - check: "lib/compiler"
Why is lib
broken up into subdirectories instead of just checking lib/
.github/workflows/clang-format-check.yml
line 13 at r1 (raw file):
- check: "lib/compiler" - check: "lib/ffi" - check: "lib/fmt"
Should not be included (this actually shouldn't
.github/workflows/clang-format-check.yml
line 17 at r1 (raw file):
- check: "lib/op-attrs" - check: "lib/pcg" - check: "lib/runtime/ffi"
Why is runtime broken up into these subdirectories?
.github/workflows/clang-format-check.yml
line 22 at r1 (raw file):
- check: "lib/runtime/test" - check: "lib/substitutions" - check: "lib/triton"
triton
should be ignored (it's third-party code)
.github/workflows/clang-format-check.yml
line 24 at r1 (raw file):
- check: "lib/triton" - check: "lib/utils" - check: "python"
Ignore python
directory as it will eventually be removed in repo-refactor
.github/workflows/clang-format-check.yml
line 25 at r1 (raw file):
- check: "lib/utils" - check: "python" - check: "scripts"
I don't think there are any C/C++ programs in scripts
.github/workflows/clang-format-check.yml
line 29 at r1 (raw file):
- check: "examples" - check: "bindings" - check: "config"
I don't think there are any C/C++ programs in config
.github/workflows/clang-format-check.yml
line 30 at r1 (raw file):
- check: "bindings" - check: "config" - check: "deps"
deps
is third-party code and should not be checked
.github/workflows/clang-format-check.yml
line 31 at r1 (raw file):
- check: "config" - check: "deps" - check: "packaging"
I don't think there are any C/C++ programs in packaging
.github/workflows/clang-format-check.yml
line 32 at r1 (raw file):
- check: "deps" - check: "packaging" - check: "substitutions"
There aren't any C/C++ programs in substitutions
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.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @aquan9)
.github/workflows/clang-format-check.yml
line 11 at r1 (raw file):
Previously, lockshaw (Colin Unger) wrote…
Why is
lib
broken up into subdirectories instead of just checkinglib/
Just realized you mentioned this in the PR description, but I'm not sure I understand how this speeds things up--are the different checks done in parallel?
Different checks appear to be executed as separate tasks in parallel, and when I was watching them the directories in lib are the ones that took the longest which is why I broke it up into separate checks. I'll remove the set of directories that you mentioned don't need to be checked. |
Here is the documentation for CI job matrices that describes their parallelization approach: https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs |
Description of changes:
Add format checking back into repo-refactor.
I updated the list of directories that it checks, and broke up the directories in lib into separate checks for a little bit more speed.
I disabled fail-fast so that you can get a thorough report of everything that is incorrectly formatted.
Related Issues:
Linked Issues:
Issues closed by this PR:
N/A
Before merging:
N/A
This change is