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

make phoebus a library #206

Merged
merged 4 commits into from
Mar 12, 2024
Merged

make phoebus a library #206

merged 4 commits into from
Mar 12, 2024

Conversation

Yurlungur
Copy link
Collaborator

PR Summary

Similar to what we do in riot. This has several advantages:

  1. Unit test code can link against Phoebus, significantly simplifying the cmake workflow
  2. Other codes can slurp in Phoebus, if this is desired.

@brryan and @bprather this might be useful for you. And @mari2895 this should solve the issue in #205 .

PR Checklist

  • Adds a test for any bugs fixed. Adds tests for new features.
  • Format your changes by calling scripts/bash/format.sh.
  • Explain what you did.

@Yurlungur Yurlungur added the enhancement New feature or request label Mar 12, 2024
@Yurlungur Yurlungur self-assigned this Mar 12, 2024
mari2895
mari2895 approved these changes Mar 12, 2024
View reviewed changes
Copy link
Collaborator

@mari2895 mari2895 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

lroberts36
lroberts36 approved these changes Mar 12, 2024
View reviewed changes
Copy link
Collaborator

@lroberts36 lroberts36 left a comment

Choose a reason for hiding this comment

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

LGTM

@jonahm-LANL
properly link con2prim (no idea how that was working before) and also…
3e5190b 
… switch test to explicitly use analytic minkowski
AstroBarker
AstroBarker approved these changes Mar 12, 2024
View reviewed changes
Copy link
Collaborator

@AstroBarker AstroBarker left a comment

Choose a reason for hiding this comment

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

All tests now passing locally, LGTM

@Yurlungur Yurlungur merged commit e9fd6c2 into main Mar 12, 2024
2 checks passed
@Yurlungur Yurlungur deleted the jmm/makelib branch March 12, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mari2895 mari2895 mari2895 approved these changes

@AstroBarker AstroBarker AstroBarker approved these changes

@lroberts36 lroberts36 lroberts36 approved these changes

@brryan brryan Awaiting requested review from brryan

@bprather bprather Awaiting requested review from bprather

@jdolence jdolence Awaiting requested review from jdolence

@curiousmiah curiousmiah Awaiting requested review from curiousmiah

@pdmullen pdmullen Awaiting requested review from pdmullen

Assignees

@Yurlungur Yurlungur

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Yurlungur @lroberts36 @AstroBarker @mari2895 @jonahm-LANL