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

Remove unused-but-set variables in velox/common/caching/tests/SsdFileTest.cpp +15 #9702

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

r-barnes
Copy link
Contributor

@r-barnes r-barnes commented May 3, 2024

Summary:
This diff removes a variable that was set, but which was not used.

LLVM-15 has a warning -Wunused-but-set-variable which we treat as an error because it's so often diagnostic of a code issue. Unused but set variables often indicate a programming mistake, but can also just be unnecessary cruft that harms readability and performance.

Removing this variable will not change how your code works, but the unused variable may indicate your code isn't working the way you thought it was. I've gone through each of these by hand, but mistakes may have slipped through. If you feel the diff needs changes before landing, please commandeer and make appropriate changes: there are hundreds of these and responding to them individually is challenging.

For questions/comments, contact r-barnes.

  • If you approve of this diff, please use the "Accept & Ship" button :-)

Differential Revision: D56887334

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 3, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56887334

Copy link

netlify bot commented May 3, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 80927c7
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/663d3a0561e97b00088b6bfa

r-barnes added a commit to r-barnes/velox that referenced this pull request May 3, 2024
…Test.cpp +15 (facebookincubator#9702)

Summary:

This diff removes a variable that was set, but which was not used.

LLVM-15 has a warning `-Wunused-but-set-variable` which we treat as an error because it's so often diagnostic of a code issue. Unused but set variables often indicate a programming mistake, but can also just be unnecessary cruft that harms readability and performance.

Removing this variable will not change how your code works, but the unused variable may indicate your code isn't working the way you thought it was. I've gone through each of these by hand, but mistakes may have slipped through. If you feel the diff needs changes before landing, **please commandeer** and make appropriate changes: there are hundreds of these and responding to them individually is challenging.

For questions/comments, contact r-barnes.

 - If you approve of this diff, please use the "Accept & Ship" button :-)

Differential Revision: D56887334
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56887334

…Test.cpp +15 (facebookincubator#9702)

Summary:
Pull Request resolved: facebookincubator#9702

This diff removes a variable that was set, but which was not used.

LLVM-15 has a warning `-Wunused-but-set-variable` which we treat as an error because it's so often diagnostic of a code issue. Unused but set variables often indicate a programming mistake, but can also just be unnecessary cruft that harms readability and performance.

Removing this variable will not change how your code works, but the unused variable may indicate your code isn't working the way you thought it was. I've gone through each of these by hand, but mistakes may have slipped through. If you feel the diff needs changes before landing, **please commandeer** and make appropriate changes: there are hundreds of these and responding to them individually is challenging.

For questions/comments, contact r-barnes.

 - If you approve of this diff, please use the "Accept & Ship" button :-)

Reviewed By: palmje, dmm-fb

Differential Revision: D56887334
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56887334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants