-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: Update to the latest version of yscope-dev-utils: #63
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
Changes from 12 commits
55d0113
0f092ec
beabd86
d45ab6a
ebc90c0
7899475
10c3ead
8c1ecd8
889fef8
c5364ac
4c0cefa
148bb2d
f149b2d
cfad49e
aff82e7
35e1aa8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,5 @@ | ||
| # Lock to v19.x until we can upgrade our code to fix new v20 issues. | ||
| clang-format~=19.1 | ||
| # Lock to v19.x until we can upgrade our code to fix new v20 issues. | ||
| clang-tidy~=19.1 | ||
| clang-format>=20.1.0 | ||
| clang-tidy>=19.1.0 | ||
| colorama>=0.4.6 | ||
| gersemi>=0.16.2 | ||
| yamllint>=1.35.1 |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,8 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| version: "3" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| vars: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| G_LINT_CLANG_TIDY_DIR: "{{.G_BUILD_DIR}}/lint-clang-tidy" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Introduce clang-tidy output directory var 🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to add comment here, the variable name is self-explained. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @anlowee You're absolutely right! The variable name |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| tasks: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| cpp-check: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| desc: "Runs the C++ linters." | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -26,11 +29,11 @@ tasks: | |||||||||||||||||||||||||||||||||||||||||||||||||||
| - "cpp-configs" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "venv" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - task: ":utils:clang-format" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - task: ":utils:cpp-lint:clang-format" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| vars: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| FLAGS: "--dry-run" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| SRC_PATHS: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ref: ".G_LINT_CPP_DIRS" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| FLAGS: ["--dry-run"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| INCLUDE_FILENAME_PATTERNS: ["*.cpp", "*.h", "*.hpp"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ROOT_PATHS: ["{{.G_CPP_SRC_DIR}}"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| VENV_DIR: "{{.G_LINT_VENV_DIR}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| cpp-format-fix: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -40,11 +43,11 @@ tasks: | |||||||||||||||||||||||||||||||||||||||||||||||||||
| - "cpp-configs" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "venv" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - task: ":utils:clang-format" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - task: ":utils:cpp-lint:clang-format" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| vars: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| FLAGS: "-i" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| SRC_PATHS: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ref: ".G_LINT_CPP_DIRS" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| FLAGS: ["-i"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| INCLUDE_FILENAME_PATTERNS: ["*.cpp", "*.h", "*.hpp"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ROOT_PATHS: ["{{.G_CPP_SRC_DIR}}"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| VENV_DIR: "{{.G_LINT_VENV_DIR}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| cpp-static-check: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -69,24 +72,25 @@ tasks: | |||||||||||||||||||||||||||||||||||||||||||||||||||
| - "cpp-configs" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "venv" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - task: ":utils:clang-tidy" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - task: ":utils:cpp-lint:clang-tidy-find" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| vars: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| FLAGS: >- | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| --config-file "{{.ROOT_DIR}}/.clang-tidy" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| -p "{{.G_COMPILE_COMMANDS_DB}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| SRC_PATHS: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ref: ".G_LINT_CPP_DIRS" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| FLAGS: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "--config-file '{{.ROOT_DIR}}/.clang-tidy'" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "-p '{{.G_COMPILE_COMMANDS_DB}}'" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| INCLUDE_FILENAME_PATTERNS: ["*.cpp", "*.h", "*.hpp"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| OUTPUT_DIR: "{{.G_LINT_CLANG_TIDY_DIR}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
vars:
G_LINT_CLANG_TIDY_DIR: "{{.G_BUILD_DIR}}/lint-clang-tidy"
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am going to remove it because we didn't use it before (also there is no output dir for
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you read the docstring for clang-tidy-find, you'll see that it will default to putting it at the root of the repository. That's going to mess up developers' local copy unnecessarily, right? |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| ROOT_PATHS: ["{{.G_CPP_SRC_DIR}}"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| VENV_DIR: "{{.G_LINT_VENV_DIR}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+75
to
73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainValidate and refine clang-tidy flag syntax FLAGS:
- - "--config-file '{{.ROOT_DIR}}/.clang-tidy'"
- - "-p '{{.G_COMPILE_COMMANDS_DB}}'"
+ - "--config-file"
+ - "{{.ROOT_DIR}}/.clang-tidy"
+ - "-p"
+ - "{{.G_COMPILE_COMMANDS_DB}}"Let’s inspect how other tasks define 🏁 Script executed: #!/bin/bash
rg -n "FLAGS:" -A3 taskfilesLength of output: 1461 Separate clang-tidy flags and their arguments • Location: FLAGS:
- - "--config-file '{{.ROOT_DIR}}/.clang-tidy'"
- - "-p '{{.G_COMPILE_COMMANDS_DB}}'"
+ - "--config-file"
+ - "{{.ROOT_DIR}}/.clang-tidy"
+ - "-p"
+ - "{{.G_COMPILE_COMMANDS_DB}}"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is not clear to separate the flag and value in different lines unless the values or flag is too long There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @anlowee, I understand your preference for keeping flags and their values together for better readability. You're right that combining them in single strings like ✏️ Learnings added
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| cpp-configs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| sources: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "{{.ROOT_DIR}}/tools/yscope-dev-utils/lint-configs/.clang-format" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "{{.ROOT_DIR}}/tools/yscope-dev-utils/lint-configs/.clang-tidy" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "{{.ROOT_DIR}}/tools/yscope-dev-utils/exports/lint-configs/.clang-format" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "{{.ROOT_DIR}}/tools/yscope-dev-utils/exports/lint-configs/.clang-tidy" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "{{.TASKFILE}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| generates: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "{{.ROOT_DIR}}/.clang-format" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "{{.ROOT_DIR}}/.clang-tidy" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| dir: "{{.ROOT_DIR}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "tools/yscope-dev-utils/lint-configs/symlink-cpp-lint-configs.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "tools/yscope-dev-utils/exports/lint-configs/symlink-cpp-lint-configs.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,16 +13,24 @@ tasks: | |||||||||||||||
| - "{{.TASKFILE}}" | ||||||||||||||||
| - exclude: "{{.ROOT_DIR}}/**/build/*" | ||||||||||||||||
| - exclude: "{{.ROOT_DIR}}/**/tools/*" | ||||||||||||||||
| # The following are for Clion's default generated build dirs | ||||||||||||||||
| - exclude: "{{.ROOT_DIR}}/cmake-build-debug/*" | ||||||||||||||||
| - exclude: "{{.ROOT_DIR}}/cmake-build-release/*" | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
| dir: "{{.ROOT_DIR}}" | ||||||||||||||||
| deps: | ||||||||||||||||
| - "venv" | ||||||||||||||||
| cmds: | ||||||||||||||||
| - |- | ||||||||||||||||
| . "{{.G_LINT_VENV_DIR}}/bin/activate" | ||||||||||||||||
| find . \ | ||||||||||||||||
| \( -path '**/build' -o -path '**/tools' \) -prune -o \ | ||||||||||||||||
| \( \ | ||||||||||||||||
| -path '**/build' \ | ||||||||||||||||
| -o -path '**/tools' \ | ||||||||||||||||
| -o -path '**/cmake-build-debug' \ | ||||||||||||||||
| -o -path '**/cmake-build-release' \ | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
| \) -prune -o \ | ||||||||||||||||
| \( -iname "*.yaml" -o -iname "*.yml" \) \ | ||||||||||||||||
| -print0 | \ | ||||||||||||||||
| xargs -0 yamllint \ | ||||||||||||||||
| --config-file "tools/yscope-dev-utils/lint-configs/.yamllint.yml" \ | ||||||||||||||||
| --strict | ||||||||||||||||
| xargs -0 yamllint \ | ||||||||||||||||
| --config-file "tools/yscope-dev-utils/exports/lint-configs/.yamllint.yml" \ | ||||||||||||||||
| --strict | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
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.
Instead of using
add-package-root-to-cmake-settingsandinstall-all-finish, we should really use the newinstall-deps-and-generate-settingstask in yscope-dev-utils. You can see an example in CLP.