i#7344: Skip format checks if DISABLE_CLANG_FORMAT_CHECKS is present. #7345
Conversation
| @@ -0,0 +1,64 @@ | |||
| # ********************************************************** | |||
There was a problem hiding this comment.
Skip clang-format checks and vera tests of DISABLE_CLANG_FORMAT_CHECKS appears in one of the commit messages.
typo: s/tests of/tests if/
| @@ -0,0 +1,64 @@ | |||
| # ********************************************************** | |||
There was a problem hiding this comment.
This feature enables us to copy code over without format changes as a base line and use subsequent PRs to fix format issues.
This would be more often used to add .patch files (that's where I've wanted it before) or other files where tabs are part of the format.
| @@ -0,0 +1,64 @@ | |||
| # ********************************************************** | |||
There was a problem hiding this comment.
"gh api" is not installed on the aarchxx machines. Adding "gh" to the install list didn't work, and manually installing gh or github-cli failed using tmate. I have added a TODO in .github/workflows/ci-aarchxx.yml to add the support once "gh api" is installed.
Did you contact @AssadHashmi or @jackgallagher-arm or others to request installation of gh api?
| @@ -0,0 +1,64 @@ | |||
| # ********************************************************** | |||
There was a problem hiding this comment.
.github/actions/is-clang-format-checks-disabled/action.yml
The file name is ungrammatical: s/is/are/
| # OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH | ||
| # DAMAGE. | ||
|
|
||
| # Github Actions workflow for x86 Continuous Integration testing. |
There was a problem hiding this comment.
Stale comment: this is not a workfow nor for x86.
| RECURSE | ||
| EXCLUSION "${PROJECT_SOURCE_DIR}/make/style_checks/exclude" | ||
| ROOT "${PROJECT_SOURCE_DIR}/make/style_checks") | ||
| if (DEFINED ENV{DISABLE_CLANG_FORMAT_CHECKS} AND "$ENV{DISABLE_CLANG_FORMAT_CHECKS}" STREQUAL "yes") |
There was a problem hiding this comment.
style: Line too long; please split into 2 lines
| EXCLUSION "${PROJECT_SOURCE_DIR}/make/style_checks/exclude" | ||
| ROOT "${PROJECT_SOURCE_DIR}/make/style_checks") | ||
| if (DEFINED ENV{DISABLE_CLANG_FORMAT_CHECKS} AND "$ENV{DISABLE_CLANG_FORMAT_CHECKS}" STREQUAL "yes") | ||
| message(STATUS "vera++ code style checks are disabled") |
There was a problem hiding this comment.
It seems a little odd to have a variable about clang-format control vera++: should it be DISABLE_FORMAT_CHECKS instead to be more general?
| string(REGEX MATCH "\n[^\n]*\t[^\n]*" match "${diff_notabs}") | ||
| message(FATAL_ERROR "ERROR: diff contains tabs: ${match}") | ||
| # Check for tabs other than on the revision lines. | ||
| # The clang-format check will now find these in C files, but not non-C files. |
There was a problem hiding this comment.
This is also not clang-format: so removing "clang format" from the names seems more appealing.
| add_test(vera ${VERA++_EXECUTABLE} --root "${PROJECT_SOURCE_DIR}/make/style_checks" | ||
| --error "${PROJECT_SOURCE_DIR}/make/style_checks/style_test.c") | ||
| set_tests_properties(vera PROPERTIES PASS_REGULAR_EXPRESSION "${vera_regex}") | ||
| if (DEFINED ENV{DISABLE_CLANG_FORMAT_CHECKS} AND "$ENV{DISABLE_CLANG_FORMAT_CHECKS}" STREQUAL "yes") |
There was a problem hiding this comment.
Seems better to have this be a CMake variable/option at this point in the CMake code. Have the top-level CMake declare an option and have its default value be this environment variable, then use the option here.
| # remember to add it to the send-failure-notification job to ensure failure | ||
| # notifications are set up properly. | ||
|
|
||
| # Check if DISABLE_CLANG_FORMAT_CHECKS appears in the PR commit messages. |
There was a problem hiding this comment.
See other comment as well: since this controls not just clang-format but vera++ and tab checks, I would rename to DISABLE_FORMAT_CHECKS and rename this file and all the variable and step names as well.
Skip clang-format checks and vera tests of DISABLE_CLANG_FORMAT_CHECKS appears in one of the commit messages.
This feature enables us to copy code over without format changes as a base line and use subsequent PRs to fix format issues.
Test: I added DISABLE_CLANG_FORMAT_CHECKS as part of a commit message to #7304 and verified format checks were disabled except aarch64 build.
"gh api" is not installed on the aarchxx machines. Adding "gh" to the install list didn't work, and manually installing gh or github-cli failed using tmate. I have added a TODO in .github/workflows/ci-aarchxx.yml to add the support once "gh api" is installed.
Issue: #7344