Skip to content

MINOR: [C++][CMake] Remove unused CMAKE_SKIP_INSTALL_ALL_DEPENDENCY - #41332

Merged
wgtmac merged 1 commit into
apache:mainfrom
wgtmac:remove_unused_cmake
Apr 22, 2024
Merged

MINOR: [C++][CMake] Remove unused CMAKE_SKIP_INSTALL_ALL_DEPENDENCY#41332
wgtmac merged 1 commit into
apache:mainfrom
wgtmac:remove_unused_cmake

Conversation

@wgtmac

Copy link
Copy Markdown
Member

Rationale for this change

CMAKE_SKIP_INSTALL_ALL_DEPENDENCY was removed in #75 but it seems that there is still one line remaining.

What changes are included in this PR?

Remove unused CMAKE_SKIP_INSTALL_ALL_DEPENDENCY.

Are these changes tested?

Pass all CIs.

Are there any user-facing changes?

No.

@wgtmac

Copy link
Copy Markdown
MemberAuthor

I found this is not cleaned up when reading the CMake code. cc @kou

@koukou changed the title MINOR: [CMake] Remove unused CMAKE_SKIP_INSTALL_ALL_DEPENDENCYMINOR: [C++][CMake] Remove unused CMAKE_SKIP_INSTALL_ALL_DEPENDENCYApr 22, 2024
kou
kou approved these changes Apr 22, 2024

@koukou left a comment

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.

+1

Good catch!

@github-actionsgithub-actionsBot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Apr 22, 2024
@wgtmac
wgtmac merged commit 08aefc3 into apache:mainApr 22, 2024
@wgtmacwgtmac removed the awaiting merge Awaiting merge label Apr 22, 2024
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 08aefc3.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 9 possible false positives for unstable benchmarks that are known to sometimes produce them.

zanmato1984 pushed a commit to zanmato1984/arrow that referenced this pull request Apr 24, 2024
…pache#41332)
### Rationale for this change
CMAKE_SKIP_INSTALL_ALL_DEPENDENCY was removed in apache#75 but it seems that there is still one line remaining.
### What changes are included in this PR?
Remove unused CMAKE_SKIP_INSTALL_ALL_DEPENDENCY.
### Are these changes tested?
Pass all CIs.
### Are there any user-facing changes?
No.
Authored-by: Gang Wu <ustcwg@gmail.com>
Signed-off-by: Gang Wu <ustcwg@gmail.com>
vibhatha pushed a commit to vibhatha/arrow that referenced this pull request May 25, 2024
…pache#41332)
### Rationale for this change
CMAKE_SKIP_INSTALL_ALL_DEPENDENCY was removed in apache#75 but it seems that there is still one line remaining.
### What changes are included in this PR?
Remove unused CMAKE_SKIP_INSTALL_ALL_DEPENDENCY.
### Are these changes tested?
Pass all CIs.
### Are there any user-facing changes?
No.
Authored-by: Gang Wu <ustcwg@gmail.com>
Signed-off-by: Gang Wu <ustcwg@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@wgtmac@kou