Skip to content
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

Merged
merged 10 commits into from
Aug 8, 2023
Merged

Ci update #898

merged 10 commits into from
Aug 8, 2023

Conversation

aquan9
Copy link
Collaborator

@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

@aquan9 aquan9 requested a review from lockshaw July 28, 2023 18:34
@jiazhihao jiazhihao added the repo-refactor Topics related to the repo and search refactors label Jul 28, 2023
Copy link
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
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
Collaborator 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
Collaborator 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

@lockshaw lockshaw merged commit 3eade8f into flexflow:repo-refactor Aug 8, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repo-refactor Topics related to the repo and search refactors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants