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.

Sign up for GitHub

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

Jump to bottom

algorithms_test/CMakeLists.txt: alternative option detection mechanism #1132

Merged
merged 1 commit into from
Nov 21, 2023
Merged

algorithms_test/CMakeLists.txt: alternative option detection mechanism #1132

merged 1 commit into from
Nov 21, 2023

Conversation

veprbl
Copy link
Member

@veprbl veprbl commented Nov 21, 2023

Briefly, what does this PR introduce?

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

Does this PR change default behavior?

@veprbl veprbl requested a review from wdconinc November 21, 2023 09:44
@veprbl veprbl requested a review from bleaktwig November 21, 2023 09:44
@veprbl
Copy link
Member Author

veprbl commented Nov 21, 2023

I may have tried to use a feature [1] from an unreleased version of cmake.
https://cmake.org/cmake/help/git-master/variable/CMAKE_LANG_COMPILER_LINKER_ID.html#variable:CMAKE_%3CLANG%3E_COMPILER_LINKER_ID

bleaktwig
bleaktwig approved these changes Nov 21, 2023
View reviewed changes
Copy link

@bleaktwig bleaktwig left a comment

Choose a reason for hiding this comment

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

Very clean solution, the repo now compiles on my environment.

@github-actions GitHub Actions
Copy link

Capybara summary for PR 1132

@wdconinc wdconinc added this pull request to the merge queue Nov 21, 2023
Merged via the queue into main with commit 87c3a26 Nov 21, 2023
74 checks passed
@wdconinc wdconinc deleted the pr/cmake_alt_no_as_needed branch November 21, 2023 14:15
@wdconinc
Copy link
Contributor

Capybara summary for PR 1132

I realized that capybara reports lump the gcc and clang test run results together, which is not ideal. Maybe we only need to run clang, not gcc.

@wdconinc wdconinc mentioned this pull request Nov 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bleaktwig bleaktwig bleaktwig approved these changes

@wdconinc wdconinc wdconinc approved these changes

Assignees
No one assigned
Labels
topic: infrastructure
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@veprbl @wdconinc @bleaktwig