Skip to content

Ci update#898

Merged
lockshaw merged 10 commits intoflexflow:repo-refactorfrom
aquan9:ci-update
Aug 8, 2023
Merged

Ci update#898
lockshaw merged 10 commits intoflexflow:repo-refactorfrom
aquan9:ci-update

Conversation

@aquan9
Copy link
Copy Markdown
Contributor

@aquan9 aquan9 commented Jul 28, 2023

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:

  • Did you update the flexflow-third-party repo, if modifying any of the Cmake files, the build configs, or the submodules?

N/A


This change is Reviewable

Copy link
Copy Markdown
Collaborator

@lockshaw lockshaw left a 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

Copy link
Copy Markdown
Collaborator

@lockshaw lockshaw left a 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 checking lib/

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?

@aquan9
Copy link
Copy Markdown
Contributor Author

aquan9 commented Jul 31, 2023

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.

@aquan9 aquan9 requested a review from lockshaw July 31, 2023 04:47
@aquan9
Copy link
Copy Markdown
Contributor Author

aquan9 commented Jul 31, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants