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

chore: Enable -Werror=unused-parameter flag #2507

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Conversation

c8ef
Copy link
Contributor

@c8ef c8ef commented Aug 26, 2024

@@ -106,11 +106,6 @@ constexpr void SEED(const T x0, const T x1, const T x2) {
SET3(x, x0, x1, x2), SET3(a, A0, A1, A2), c = C;
}

template <typename T>
constexpr T HiBit(const T x) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function appears to be unused.

Copy link
Member

Choose a reason for hiding this comment

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

this is vendored ( pasted from other project )

Maybe we can disable lint for "vendor" dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even though it's in the vendored directory, I believe it's more of a modern C++ re-implementation based on the comment below. Therefore, removing the unused function should be acceptable, at least NFC.

/*
* Rewritten in C++ with constexprs and templates by @tanruixiang in
* https://github.com/apache/kvrocks/commit/900e0f44781d9459649e9864ba09abe6000ce987
*/

@git-hulk git-hulk requested a review from torwig August 26, 2024 15:20
@c8ef
Copy link
Contributor Author

c8ef commented Aug 27, 2024

I don't know why this test failed. I believe the entire patch should be NFC. Other CI failures have already been fixed in the latest patch. Please allow the CI to run.

@c8ef c8ef requested a review from mapleFU August 27, 2024 00:39
Copy link

sonarcloud bot commented Aug 27, 2024

@mapleFU mapleFU merged commit fa3290c into apache:unstable Aug 27, 2024
31 checks passed
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.

Treat unused-parameter warnings as errors in CMake
4 participants