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

Adding the functionality to include analysis specific c++ code to CROWN #288

Merged
merged 10 commits into from
Jan 16, 2025

Conversation

a-monsch
Copy link
Contributor

@a-monsch a-monsch commented Dec 10, 2024

Addition of functionality to offload c++ specific code to analysis specific configurations stored in cpp_addons/include and cpp_addons/src

Things that need to be done:

  • Documentation on what to look for when using this functionality
  • finding a better name?
  • checking if this is a good enough solution

Copy link
Contributor

@tvoigtlaender tvoigtlaender left a comment

Choose a reason for hiding this comment

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

The included directories should be as restricted as possible.

Also, the downstream effects of this change should be considered.
To me, it looks like KingMaker is not affected by this change.
It should be tested anyway to make sure nothing breaks. Especially because KingMaker currently only clones the newest version of CROWN during setup, not a specific commit.

cmake/ConfigureCrownlib.cmake Outdated Show resolved Hide resolved
cmake/ConfigureCrownlib.cmake Outdated Show resolved Hide resolved
@tvoigtlaender
Copy link
Contributor

Added PR to this branch and KingMaker main branch.

@a-monsch a-monsch marked this pull request as ready for review January 14, 2025 11:32
Copy link
Contributor

@nshadskiy nshadskiy left a comment

Choose a reason for hiding this comment

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

All fine from my side.
We seem to have an issue with the build checks (authentication to get jsonpog repo), but this is unrelated to the PR.

@nshadskiy
Copy link
Contributor

The issue with the authentication is resolved for now. To get the jsonpog repository we need a valid private ssh key to access CERN gitlab. The previous one was probably Sebastians, so I guess it expired. I now added my ssh key.

@nshadskiy nshadskiy merged commit 3120dde into main Jan 16, 2025
7 checks passed
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.

5 participants