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

fix: install IActsGeoSvc.h into <prefix>/include/k4ActsTracking/IActsGeoSvc.h #18

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

wdconinc
Copy link

This installs the interface header in the right place for use by downstream projects.

BEGINRELEASENOTES

  • Install interface header into suitably prefixed directory

ENDRELEASENOTES

@wdconinc
Copy link
Author

@Zehvogel FYI (not sure you have this repo setup to notify you of new PRs)

@Zehvogel Zehvogel requested a review from andresailer January 8, 2024 09:33
@Zehvogel
Copy link
Collaborator

Zehvogel commented Jan 8, 2024

Hi @wdconinc, thanks for the contribution! Can you rebase this if you get the chance?
This was somewhat further down my list as this repo is not in the key4hep-stack at the moment... so does this PR mean you plan to use it? :)

@andresailer can you take a look? It looks alright to me but my cmake knowledge is limited.

@wdconinc
Copy link
Author

wdconinc commented Jan 8, 2024

Yes, we've moved over to using k4FWCore for our Gaudi-based components, and we would like to move out Acts geo service into k4ActsTracking. Since we're already using this for Acts tracking in production, I'm hoping it will be useful for you if we upstream it.

@wdconinc wdconinc force-pushed the install-interface-header branch from 56cce3a to 27bff3d Compare January 8, 2024 14:08
@wdconinc
Copy link
Author

wdconinc commented Jan 8, 2024

Rebased.

@wdconinc
Copy link
Author

wdconinc commented Jan 8, 2024

I'll check what's going on with the failing tests, but might be a few days due to travel.

@paulgessinger
Copy link
Collaborator

The generated cmake config file does not seem to be found by the downstream cmake build.

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.

3 participants