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

Allow usage of system provided status-code #258

Merged

Conversation

BurningEnlightenment
Copy link
Collaborator

@BurningEnlightenment BurningEnlightenment commented Jan 11, 2022

Purpose

The current cmake buildsystem vendors the status-code headers which is at odds with most package managers which require using the version provided through them. Furthermore it is unergonomic for users who need to include additional categories like com_code.hpp.

Solution Sketch

Add a cmake feature toggle which instructs cmake to look for an installed status-code package. If configured that way, we would not install any status-code headers but reference the imported status-code::hl target.

Additional explanatory comments

  • I upgraded the minimum required cmake version to 3.5 as that is already required by quickcpplib. I think it should be considered to require an even newer version as a similar discussion on llvm-dev last year revealed that all current LTS platforms support at least 3.6.1 (or 3.13.4 if one is willing to sacrifice RHEL 6 and Debian 9 support).
  • The changes should be backward compatible and consumers have to opt in.

@ned14 ned14 merged commit 147ec1e into ned14:develop Jan 12, 2022
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link

Unit Test Results

  1 files   -     2  38 suites   - 144   0s ⏱️ ±0s
40 tests  -     1  40 ✔️  -     1  0 💤 ±0  0 ❌ ±0 
40 runs   - 152  40 ✔️  - 152  0 💤 ±0  0 ❌ ±0 

Results for commit 147ec1e. ± Comparison against base commit 99d1aac.

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.

2 participants