Skip to content

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.

Sign up for GitHub

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

Merged
anlowee merged 16 commits into from
May 27, 2025
Merged

refactor: Update to the latest version of yscope-dev-utils: #63

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
55d0113
:Make task build:all runnable
May 23, 2025
0f092ec
Fix build CI (Kirk figured this)
May 23, 2025
beabd86
Fix
May 23, 2025
d45ab6a
Add else check
May 23, 2025
ebc90c0
Fix unit test
May 23, 2025
7899475
Fix lint check
May 23, 2025
10c3ead
Fix lint
May 23, 2025
8c1ecd8
Fix
May 23, 2025
889fef8
Fix lint
May 24, 2025
c5364ac
Fix Kirk comments
anlowee May 26, 2025
4c0cefa
Fix
anlowee May 26, 2025
148bb2d
Fix
anlowee May 26, 2025
f149b2d
Minor fix
anlowee May 26, 2025
cfad49e
Use yscope-dev-util's cmake:install-deps-and-generate-settings to rep…
kirkrodrigues May 27, 2025
aff82e7
Fix Kirk comments
anlowee May 27, 2025
35e1aa8
Minor fix
anlowee May 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions lint-requirements.txt
View file
Open in desktop
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
20 changes: 10 additions & 10 deletions src/ystdlib/containers/test/test_Array.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,36 +75,36 @@ void test_list_initialization_in_function_call(

namespace ystdlib::containers::test {
TEST_CASE("test_array_empty", "[containers][Array]") {
Array<int> arr(0);
Array<size_t> arr(0);
REQUIRE(arr.empty());
// NOLINTNEXTLINE(readability-container-size-empty)
REQUIRE((0 == arr.size()));
REQUIRE((arr.begin() == arr.end()));
}

TEST_CASE("test_array_reference", "[containers][Array]") {
Array<int> arr(cBufferSize);
for (int idx{0}; idx < cBufferSize; ++idx) {
Array<size_t> arr(cBufferSize);
for (size_t idx{0}; idx < cBufferSize; ++idx) {
arr.at(idx) = idx;
}
auto const& arr_const_ref = arr;
REQUIRE(std::ranges::equal(arr, arr_const_ref));
}

TEST_CASE("test_array_ranged_copy", "[containers][Array]") {
std::vector<int> vec;
for (int idx{0}; idx < cBufferSize; ++idx) {
std::vector<size_t> vec;
for (size_t idx{0}; idx < cBufferSize; ++idx) {
vec.push_back(idx);
}
Array<int> arr(cBufferSize);
Array<size_t> arr(cBufferSize);
std::ranges::copy(vec, arr.begin());
REQUIRE(std::ranges::equal(vec, arr));
}

TEST_CASE("test_array_movable", "[containers][Array]") {
Array<int> arr(cBufferSize);
Array<int> reference_array(cBufferSize);
for (int idx{0}; idx < cBufferSize; ++idx) {
Array<size_t> arr(cBufferSize);
Array<size_t> reference_array(cBufferSize);
for (size_t idx{0}; idx < cBufferSize; ++idx) {
arr.at(idx) = idx;
reference_array.at(idx) = idx;
}
Expand All @@ -113,7 +113,7 @@ TEST_CASE("test_array_movable", "[containers][Array]") {
}

TEST_CASE("test_array_illegal_access", "[containers][Array]") {
Array<int> arr(cBufferSize);
Array<size_t> arr(cBufferSize);
REQUIRE_THROWS_AS(arr.at(-1), std::out_of_range);
REQUIRE_THROWS_AS(arr.at(cBufferSize), std::out_of_range);
}
Expand Down
2 changes: 1 addition & 1 deletion src/ystdlib/error_handling/ErrorCode.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class ErrorCode {
/**
* @return The reference to the singleton of the corresponded error category.
*/
[[nodiscard]] constexpr static auto get_category() -> ErrorCategory<ErrorCodeEnum> const& {
[[nodiscard]] static constexpr auto get_category() -> ErrorCategory<ErrorCodeEnum> const& {
return cCategory;
}

Expand Down
4 changes: 2 additions & 2 deletions src/ystdlib/error_handling/test/test_Result.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ TEST_CASE("test_result_unique_ptr", "[error_handling][Result]") {

auto const result_has_error{cUniquePtrFunc(true)};
REQUIRE(result_has_error.has_error());
REQUIRE(AlwaysSuccessErrorCode{AlwaysSuccessErrorCodeEnum::Success} == result_has_error.error()
);
REQUIRE(AlwaysSuccessErrorCode{AlwaysSuccessErrorCodeEnum::Success}
== result_has_error.error());
}

TEST_CASE("test_result_unique_ptr_in_main", "[error_handling][Result]") {
Expand Down
2 changes: 1 addition & 1 deletion taskfile.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ includes:
build: "./taskfiles/build.yaml"
deps: "./taskfiles/deps.yaml"
lint: "./taskfiles/lint.yaml"
utils: "tools/yscope-dev-utils/taskfiles/utils.yaml"
utils: "tools/yscope-dev-utils/exports/taskfiles/utils/utils.yaml"

vars:
G_BUILD_DIR: "{{.ROOT_DIR}}/build"
Expand Down
8 changes: 4 additions & 4 deletions taskfiles/build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ tasks:
deps:
- "init"
cmds:
- task: ":utils:cmake-build"
- task: ":utils:cmake:build"
vars:
BUILD_DIR: "{{.G_BUILD_DIR}}"

Expand All @@ -21,7 +21,7 @@ tasks:
deps:
- "init"
cmds:
- task: ":utils:cmake-build"
- task: ":utils:cmake:build"
vars:
BUILD_DIR: "{{.G_BUILD_DIR}}"
TARGETS:
Expand Down Expand Up @@ -49,7 +49,7 @@ tasks:
clean:
desc: "Removes all built artifacts."
deps:
- task: ":utils:cmake-clean"
- task: ":utils:cmake:clean"
vars:
BUILD_DIR: "{{.G_BUILD_DIR}}"

Expand All @@ -59,7 +59,7 @@ tasks:
- ":deps:install-all"
run: "once"
cmds:
- task: ":utils:cmake-generate"
- task: ":utils:cmake:generate"
vars:
BUILD_DIR: "{{.G_BUILD_DIR}}"
SOURCE_DIR: "{{.ROOT_DIR}}"
28 changes: 14 additions & 14 deletions taskfiles/deps.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ tasks:
internal: true
run: "once"
cmds:
- task: ":utils:cmake-install-remote-tar"
- task: ":utils:cmake:install-remote-tar"
vars:
NAME: "{{.G_CATCH2_LIB_NAME}}"
CMAKE_PACKAGE_NAME: "{{.G_CATCH2_LIB_NAME}}"
WORK_DIR: "{{.G_CATCH2_WORK_DIR}}"
FILE_SHA256: "1ab2de20460d4641553addfdfe6acd4109d871d5531f8f519a52ea4926303087"
URL: "https://github.com/catchorg/Catch2/archive/refs/tags/v3.8.0.tar.gz"
TAR_SHA256: "1ab2de20460d4641553addfdfe6acd4109d871d5531f8f519a52ea4926303087"
TAR_URL: "https://github.com/catchorg/Catch2/archive/refs/tags/v3.8.0.tar.gz"
- task: "add-package-root-to-cmake-settings"
vars:
NAME: "{{.G_CATCH2_LIB_NAME}}"
Expand All @@ -75,13 +75,13 @@ tasks:
deps:
- "install-quickcpplib"
cmds:
- task: ":utils:cmake-install-remote-tar"
- task: ":utils:cmake:install-remote-tar"
vars:
NAME: "{{.G_OUTCOME_LIB_NAME}}"
CMAKE_PACKAGE_NAME: "{{.G_OUTCOME_LIB_NAME}}"
WORK_DIR: "{{.G_OUTCOME_WORK_DIR}}"
FILE_SHA256: "0382248cbb00806ce4b5f3ce6939797dc3b597c85fd3531614959e31ef488b39"
URL: "https://github.com/ned14/outcome/archive/refs/tags/v2.2.11.tar.gz"
GEN_ARGS:
TAR_SHA256: "0382248cbb00806ce4b5f3ce6939797dc3b597c85fd3531614959e31ef488b39"
TAR_URL: "https://github.com/ned14/outcome/archive/refs/tags/v2.2.11.tar.gz"
CMAKE_GEN_ARGS:
- "-C {{.G_DEPS_CMAKE_SETTINGS_DIR}}/{{.G_QUICKCPPLIB_LIB_NAME}}.cmake"
- "-DBUILD_TESTING=OFF"
- "-DCMAKE_BUILD_TYPE=Release"

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we alphabetize these variables?

Expand All @@ -95,13 +95,13 @@ tasks:
internal: true
run: "once"
cmds:
- task: ":utils:cmake-install-remote-tar"
- task: ":utils:cmake:install-remote-tar"
vars:
NAME: "{{.G_QUICKCPPLIB_LIB_NAME}}"
CMAKE_PACKAGE_NAME: "{{.G_QUICKCPPLIB_LIB_NAME}}"
WORK_DIR: "{{.G_QUICKCPPLIB_WORK_DIR}}"
FILE_SHA256: "5d4c9b2d6fa177d3fb14f3fe3086867e43b44f4a7a944eb10ee4616b2b0f3c05"
URL: "https://github.com/ned14/quickcpplib/archive/f3e452e.tar.gz"
GEN_ARGS:
TAR_SHA256: "5d4c9b2d6fa177d3fb14f3fe3086867e43b44f4a7a944eb10ee4616b2b0f3c05"
TAR_URL: "https://github.com/ned14/quickcpplib/archive/f3e452e.tar.gz"
CMAKE_GEN_ARGS:
- "-DBUILD_TESTING=OFF"
- "-DCMAKE_BUILD_TYPE=Release"

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we alphabetize these variables?

- task: "add-package-root-to-cmake-settings"

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

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-settings and install-all-finish, we should really use the new install-deps-and-generate-settings task in yscope-dev-utils. You can see an example in CLP.

Expand Down
45 changes: 21 additions & 24 deletions taskfiles/lint-cpp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,18 @@ tasks:
desc: "Runs the C++ linters that identify formatting issues."
sources: &cpp_format_src_files
- "{{.G_LINT_VENV_CHECKSUM_FILE}}"
- "{{.G_CPP_SRC_DIR}}/**/*.cpp"
- "{{.G_CPP_SRC_DIR}}/**/*.h"
- "{{.G_CPP_SRC_DIR}}/**/*.hpp"
- "{{.G_CPP_SRC_DIR}}"

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's undo this change since ROOT_PATHS should really just be set to ["{{.G_CPP_SRC_DIR}}"].

@anlowee anlowee May 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea true. I was doing so but finally I thought we might want to also check other files under other dirs and clang-tidy will help to filter files by extension automatically lol.

- "{{.ROOT_DIR}}/.clang-format"
- "{{.TASKFILE}}"
deps:
- "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", "*.inc"]

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
INCLUDE_FILENAME_PATTERNS: ["*.cpp", "*.h", "*.hpp", "*.inc"]
INCLUDE_FILENAME_PATTERNS: ["*.cpp", "*.h", "*.hpp"]

There are no .inc files in this repo.

ROOT_PATHS: *cpp_format_src_files

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ROOT_PATHS: *cpp_format_src_files
ROOT_PATHS: ["{{.G_CPP_SRC_DIR}}"]

VENV_DIR: "{{.G_LINT_VENV_DIR}}"

cpp-format-fix:
Expand All @@ -40,11 +38,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", "*.inc"]
ROOT_PATHS: *cpp_format_src_files
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
INCLUDE_FILENAME_PATTERNS: ["*.cpp", "*.h", "*.hpp", "*.inc"]
ROOT_PATHS: *cpp_format_src_files
INCLUDE_FILENAME_PATTERNS: ["*.cpp", "*.h", "*.hpp"]
ROOT_PATHS: ["{{.G_CPP_SRC_DIR}}"]

VENV_DIR: "{{.G_LINT_VENV_DIR}}"

cpp-static-check:
Expand All @@ -55,38 +53,37 @@ tasks:
aliases:
- "cpp-static-fix"
desc: "Runs the C++ static analyzers. Only checks for warnings and violations."
sources:
sources: &cpp_static_src_files
- "{{.G_CMAKE_CACHE}}"
- "{{.G_COMPILE_COMMANDS_DB}}"
- "{{.G_LINT_VENV_CHECKSUM_FILE}}"
- "{{.G_CPP_SRC_DIR}}/**/*.cpp"
- "{{.G_CPP_SRC_DIR}}/**/*.h"
- "{{.G_CPP_SRC_DIR}}/**/*.hpp"
- "{{.G_CPP_SRC_DIR}}"
- "{{.ROOT_DIR}}/.clang-tidy"
- "{{.TASKFILE}}"

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy-find is already idempotent, so there's no real need to track sources for idempotency. So I'd suggest getting rid of sources here.

@kirkrodrigues kirkrodrigues May 27, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant get rid of the sources KV pair entirely.

deps:
- ":build:init"
- "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_PATTERNS: ["*.cpp", "*.h", "*.hpp", "*.inc"]

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
INCLUDE_PATTERNS: ["*.cpp", "*.h", "*.hpp", "*.inc"]
INCLUDE_PATTERNS: ["{{.G_CPP_SRC_DIR}}/*"]

INCLUDE_PATTERNS is different than INCLUDE_FILENAME_PATTERNS.

@anlowee anlowee May 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel we should change this line to INCLUDE_FILENAME_PATTERNS: ["*.cpp", "*.h", "*.hpp"]? (Since this is a cpp-static-check task and from the doc of clang-tidy-find it is for:

Runs clang-tidy in parallel on C++ source files that match the given filter criteria.

@kirkrodrigues kirkrodrigues May 27, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but that's already the default in clang-tidy-find, so do we need to specify it?

OUTPUT_DIR: "{{.G_LINT_CLANG_TIDY_DIR}}"

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

G_LINT_CLANG_TIDY_DIR is undefined. You probably want to add the following at the top of the file (after version ):

vars:
  G_LINT_CLANG_TIDY_DIR: "{{.G_BUILD_DIR}}/lint-clang-tidy"

@anlowee anlowee May 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 cpp=format=check/fix

@kirkrodrigues kirkrodrigues May 26, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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: *cpp_static_src_files

@kirkrodrigues kirkrodrigues May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ROOT_PATHS: *cpp_static_src_files
ROOT_PATHS: ["{{.G_CPP_SRC_DIR}}"]

VENV_DIR: "{{.G_LINT_VENV_DIR}}"
Comment on lines +75 to 73

@coderabbitai coderabbitai Bot May 26, 2025 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Validate and refine clang-tidy flag syntax
Currently the FLAGS list embeds both flag names and values in single strings (e.g., --config-file '{{.ROOT_DIR}}/.clang-tidy'). This can lead to quoting or parsing issues in YAML. Consider splitting them into separate list elements:

  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 FLAGS to ensure we follow existing conventions:

🏁 Script executed:

#!/bin/bash
rg -n "FLAGS:" -A3 taskfiles

Length of output: 1461

Separate clang-tidy flags and their arguments
The FLAGS list in the :utils:cpp-lint:clang-tidy-find task currently combines option names and their values into single strings, which diverges from the established pattern (see lines 34 and 48) and can break proper argument splitting at runtime. Please split each flag and its value into its own YAML entry:

• Location: taskfiles/lint-cpp.yaml (around lines 77–79)

   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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- 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}}"
ROOT_PATHS: ["{{.G_CPP_SRC_DIR}}"]
VENV_DIR: "{{.G_LINT_VENV_DIR}}"
- task: ":utils:cpp-lint:clang-tidy-find"
vars:
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}}"
ROOT_PATHS: ["{{.G_CPP_SRC_DIR}}"]
VENV_DIR: "{{.G_LINT_VENV_DIR}}"
🤖 Prompt for AI Agents 
In taskfiles/lint-cpp.yaml around lines 75 to 83, the FLAGS list combines flag
names and their values into single strings, which can cause quoting or parsing
issues. To fix this, split each flag and its argument into separate list
entries, for example, replace "--config-file '{{.ROOT_DIR}}/.clang-tidy'" with
two entries: "--config-file" and "'{{.ROOT_DIR}}/.clang-tidy'". Apply the same
for the "-p" flag and its value, ensuring each flag and its parameter are
distinct elements in the FLAGS list.

@anlowee anlowee May 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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

@coderabbitai coderabbitai Bot May 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 "--config-file '{{.ROOT_DIR}}/.clang-tidy'" is clearer and more intuitive to read, especially when the values aren't excessively long. I'll respect this coding style preference for the project.

   /|   /|  
  ( :v:  )
   |(_)|
✏️ Learnings added 
Learnt from: anlowee
PR: y-scope/ystdlib-cpp#63
File: taskfiles/lint-cpp.yaml:75-83
Timestamp: 2025-05-26T15:34:11.163Z
Learning: In the ystdlib-cpp project taskfiles, prefer keeping command-line flags and their values together in single YAML list entries (e.g., "--config-file '{{.ROOT_DIR}}/.clang-tidy'") rather than separating them into individual entries, unless the values are too long. This improves readability and maintains consistency with the project's style.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


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"
6 changes: 3 additions & 3 deletions taskfiles/lint-venv.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@ tasks:
run: "once"
deps:
- ":init"
- task: ":utils:validate-checksum"
- task: ":utils:checksum:validate"
vars:
CHECKSUM_FILE: "{{.CHECKSUM_FILE}}"
INCLUDE_PATTERNS: ["{{.OUTPUT_DIR}}"]
cmds:
- task: ":utils:create-venv"
- task: ":utils:misc:create-venv"
vars:
LABEL: "lint"
OUTPUT_DIR: "{{.OUTPUT_DIR}}"
REQUIREMENTS_FILE: "{{.ROOT_DIR}}/lint-requirements.txt"
# This command must be last
- task: ":utils:compute-checksum"
- task: ":utils:checksum:compute"
vars:
CHECKSUM_FILE: "{{.CHECKSUM_FILE}}"
INCLUDE_PATTERNS: ["{{.OUTPUT_DIR}}"]
2 changes: 1 addition & 1 deletion taskfiles/lint-yaml.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ tasks:
\( -iname "*.yaml" -o -iname "*.yml" \) \
-print0 | \
xargs -0 yamllint \
--config-file "tools/yscope-dev-utils/lint-configs/.yamllint.yml" \
--config-file "tools/yscope-dev-utils/exports/lint-configs/.yamllint.yml" \
--strict
2 changes: 1 addition & 1 deletion tools/yscope-dev-utils
Open in desktop
Submodule yscope-dev-utils updated 33 files
+1 −0 .github/CODEOWNERS
+41 −0 .github/ISSUE_TEMPLATE/bug-report.yaml
+1 −0 .github/ISSUE_TEMPLATE/config.yaml
+21 −0 .github/ISSUE_TEMPLATE/feature-request.yaml
+30 −0 .github/workflows/pr-title-checks.yaml
+37 −0 .github/workflows/unit-tests.yaml
+2 −0 .gitignore
+13 −0 README.md
+3 −3 docs/lint-tools-cpp.md
+7 −1 exports/lint-configs/.clang-format
+15 −1 exports/lint-configs/.clang-tidy
+37 −0 exports/lint-configs/.markdownlint-cli2.yaml
+0 −0 exports/lint-configs/.yamllint.yml
+3 −0 exports/lint-configs/lint-requirements.txt
+0 −0 exports/lint-configs/symlink-cpp-lint-configs.sh
+186 −0 exports/taskfiles/utils/boost.yaml
+3 −3 exports/taskfiles/utils/checksum.yaml
+251 −0 exports/taskfiles/utils/cmake.yaml
+338 −0 exports/taskfiles/utils/cpp-lint.yaml
+0 −0 exports/taskfiles/utils/misc.yaml
+69 −16 exports/taskfiles/utils/remote.yaml
+9 −0 exports/taskfiles/utils/utils.yaml
+0 −4 lint-configs/lint-requirements.txt
+19 −0 taskfile.yaml
+17 −0 taskfiles/boost/.clang-format
+54 −0 taskfiles/boost/CMakeLists.txt
+160 −0 taskfiles/boost/test_boost.cpp
+70 −0 taskfiles/boost/tests.yaml
+97 −0 taskfiles/remote/tests.yaml
+20 −0 taskfiles/tests.yaml
+0 −195 taskfiles/utils-cmake.yaml
+0 −39 taskfiles/utils-cpp-lint.yaml
+0 −18 taskfiles/utils.yaml