Skip to content

Conversation

@wgtmac
Copy link
Member

@wgtmac wgtmac commented Apr 22, 2024

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
Member Author

wgtmac commented Apr 22, 2024

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

@kou kou changed the title MINOR: [CMake] Remove unused CMAKE_SKIP_INSTALL_ALL_DEPENDENCY MINOR: [C++][CMake] Remove unused CMAKE_SKIP_INSTALL_ALL_DEPENDENCY Apr 22, 2024
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

Good catch!

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Apr 22, 2024
@wgtmac wgtmac merged commit 08aefc3 into apache:main Apr 22, 2024
@wgtmac wgtmac removed the awaiting merge Awaiting merge label Apr 22, 2024
@conbench-apache-arrow
Copy link

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 free to 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