Skip to content

chore: add readability-isolate-declaration to .clang-tidy - #419

Merged
wgtmac merged 3 commits into
apache:mainfrom
HuaHuaY:add_clang_tidy_rule
Dec 18, 2025
Merged

chore: add readability-isolate-declaration to .clang-tidy#419
wgtmac merged 3 commits into
apache:mainfrom
HuaHuaY:add_clang_tidy_rule

Conversation

@HuaHuaY

@HuaHuaY HuaHuaY commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

Add this rule to avoid defining multiple variables on the same line.

Comment thread src/iceberg/test/mock_catalog.h Outdated
#include <gtest/gtest.h>

#include "iceberg/catalog.h"
#include "iceberg/table.h"

@HuaHuaY HuaHuaY Dec 17, 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.

Fix a bug. Otherwise clang-tidy checks would fail when someone else modified this file.
The reasion is MOCK_METHOD((Result<std::unique_ptr<Table>>), ... will instantiate std::expected<std::unique_ptr<Table>> but Table here is just a forward declaration so std::unique_ptr will fail to generate destructor.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, I remember in the test file if include mock_catalog.h without including table.h, it will fail to compile.

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.

If we change std::unique_ptr<Table> to std::shared_ptr<Table> like #418, we don't need to get the complete definition.

Comment thread src/iceberg/test/mock_catalog.h Outdated
#include <gtest/gtest.h>

#include "iceberg/catalog.h"
#include "iceberg/table.h"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, I remember in the test file if include mock_catalog.h without including table.h, it will fail to compile.

@wgtmac wgtmac changed the title feat: add readability-isolate-declaration in .clang-tidy chore: add readability-isolate-declaration to .clang-tidy Dec 18, 2025
@wgtmac
wgtmac merged commit 39133d0 into apache:main Dec 18, 2025
10 checks passed
@HuaHuaY
HuaHuaY deleted the add_clang_tidy_rule branch December 18, 2025 09:35
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